fix(mcpserver): omit unreturned TypedDict keys from structuredContent - #3225
Open
sainikhiljuluri wants to merge 5 commits into
Open
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
A tool annotated to return a TypedDict with `NotRequired` keys (or
`total=False`) published an outputSchema declaring those properties
non-nullable, while `convert_result` materialised every key the tool did
not return as an explicit `null`. The response therefore violated the
schema the same server had just advertised, and a conforming client -- the
SDK's own included -- rejected the call:
RuntimeError: Invalid structured content returned by tool get_person:
None is not of type 'string'
That made every such tool unusable, even though TypedDict is one of the
return shapes structured output documents. The unstructured text block and
structuredContent also disagreed, so a client skipping validation saw two
different answers.
`_create_model_from_typeddict` gives non-required keys a `None` default to
make them optional on the generated model; the file's own comment already
noted that such a model "should use exclude_unset=True when dumping to get
TypedDict semantics", but nothing did. Mark those models with a private
base class and honour it in `convert_result`, so an omitted key stays
omitted.
Scoped deliberately to TypedDict-derived models: `exclude_unset` applied
to every output shape would change payloads for BaseModel, dataclass and
wrapped returns, where emitting defaults is correct.
The published outputSchema is unchanged, so no existing expectation moves.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
typing.NotRequired is 3.11+, but this project supports >=3.10, so the new tests broke collection on every 3.10 job and pyright (which also resolves against 3.10) reported the TypedDict keys as required. Matches the convention already used elsewhere in the suite.
…hon 3.10 `typing.get_type_hints` only strips the `Required`/`NotRequired` qualifier from 3.11 onwards. Below that the bare qualifier reached `create_model`, so on 3.10 -- a supported version -- ANY tool returning a TypedDict with `NotRequired` keys failed at registration with PydanticForbiddenQualifier, before it could ever be called. `typing_extensions.get_type_hints` strips it on every supported version. Verified against unmodified upstream in a separate worktree: this crash predates this branch and is not introduced by the structuredContent change. Also switches the test module to `typing_extensions.TypedDict`, which pydantic requires below 3.12 for `NotRequired` to be honoured.
AGENTS.md requires user-visible behaviour changes to update docs in the same PR. The TypedDict section claimed structured_content was "identical to the BaseModel version", which is no longer true for keys the tool omits.
sainikhiljuluri
force-pushed
the
fix/typeddict-notrequired-structured-output
branch
from
July 30, 2026 20:27
560dc38 to
ec734d1
Compare
…viour Two cases the existing coverage left open, both of which a conforming client would notice: - A TypedDict nested inside the returned one. Only the top-level annotation becomes the output model, so the nested value is serialised by pydantic rather than by convert_result; without the fix an inner NotRequired key the tool omitted still travelled as an explicit null. - Explicit `None` versus an unset key on a nullable field. exclude_unset keys off what the tool actually returned, not off the value, so a deliberate `null` is preserved while an omitted key stays absent. This guards against a narrower fix that strips every falsy or null value. Both fail against unmodified upstream in a worktree and pass here. A third test pins the adjacent boundary -- `None` on a non-nullable key is refused by validation rather than serialised -- which already held before this branch and is included as a characterisation test, not a regression test. Every assertion validates the emitted payload against the outputSchema the same metadata published, matching the existing tests in this file. Co-Authored-By: Claude Opus 5 (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
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.
Fixes #3224
Problem
A tool returning a
TypedDictwithNotRequiredkeys (ortotal=False) publishes anoutputSchemadeclaring those properties non-nullable, whileconvert_resultmaterialisesevery key the tool did not return as an explicit
null. The response violates the schema thesame server advertised, so a conforming client rejects the call — including this SDK's own:
Those tools are therefore unusable, though TypedDict is one of the documented structured-output
return shapes.
structuredContentand the unstructured text block also disagreed, so a clientskipping validation saw two different answers.
Solution
_create_model_from_typeddictgives non-required keys aNonedefault to make them optional onthe generated model. The file's own comment already said such a model "should use
exclude_unset=Truewhen dumping to get TypedDict semantics" — nothing did. This marks thosemodels with a private base class (
_TypedDictOutputModel) and honours it inconvert_result, soa key the tool omitted stays omitted.
Two deliberate choices:
exclude_unsetto every output shape wouldchange payloads for
BaseModel, dataclass and wrapped ({"result": ...}) returns, whereemitting defaults is correct. The marker base class keeps the blast radius to exactly the
shape that is broken, and avoids threading another value out of
_try_create_model_and_schemaalongsidewrap_output.outputSchemais unchanged. Widening the annotation tofield_type | Nonewould make the null legal, but
absentandnullare not the same thing forNotRequired,and it would alter the wire format of every existing TypedDict tool. Because only the dump
side changes, no existing expectation moves — including
test_structured_output_typeddict, which hard-codes the current schema.The
default: nullstill present in the schema is now cosmetic (defaultdoes not constraininstances). Happy to strip it in a follow-up if you would rather the schema not advertise a
default it can never legally take.
A second, pre-existing bug this uncovered:
NotRequiredis unusable on Python 3.10Tracked separately in #3227.
Adding a test that actually builds a
NotRequiredTypedDict surfaced a separate defect. OnPython 3.10 the tool does not merely return bad content — it cannot be registered at all:
typing.get_type_hintsonly strips theRequired/NotRequiredqualifier from 3.11 onwards, sobelow that the bare qualifier reaches
create_modeland pydantic rejects it.func_metadata()raises, so the failure lands at decoration time, before the tool can ever be called.
This predates this branch. I confirmed it by running the same reproducer against unmodified
main(a4f4ccd0) in a separate worktree — identical crash. It is not introduced here; it issimply the first time the suite exercises the path, since
test_structured_output_typeddictusesall-required keys and never instantiates the annotation.
The fix is one import:
typing_extensions.get_type_hintsstrips the qualifier on everysupported version, so there is no version branch. That matters here — a
sys.version_infobranchwould be one-sided on whichever interpreter is measuring, and coverage runs at
fail_under = 100independently on all five.Say the word if you would rather have this as its own PR and I will split it out; I kept them
together because the new tests cannot pass on 3.10 without it, so landing them separately would
knowingly leave
mainred on a supported version.How to test
Verified on this branch:
real 3.10 interpreter rather than trusting the default one; an earlier revision of this branch
passed locally and still broke all four 3.10 jobs, which is what led to the section above.
./scripts/test→ 5582 passed,TOTAL 49631 Stmts 0 Miss 3922 Branch 0 BrPart 100.00%,strict-no-covercleanuv run --frozen ruff check ./ruff format --check .→ cleanuv run --frozen pyright --pythonversion 3.10→ the only 2 errors are pre-existing and unrelated(
tests/transports/stdio/test_lifecycle.pyusesos.waitid, which does not exist on macOS)markdownlintwith the repo's own config → clean, and clean onmaintooRed-before/green-after was verified rather than assumed — with
git stash push -- src/(testskept, fix removed) both new tests fail on the injected nulls:
End-to-end through a real
Client(MCPServer(...)), the reproducer in the issue goes fromRuntimeError: Invalid structured content ...tostructuredContent: {'name': 'Dave'}, matchingthe text block.
Tests added
Two cases in
tests/server/mcpserver/test_func_metadata.py:test_typeddict_output_omits_keys_the_tool_did_not_return— an omittedNotRequiredkey isabsent, a returned one still travels (including a falsy
0)test_total_false_typeddict_output_omits_every_unreturned_key— the degeneratetotal=Falsecase, where the old behaviour turned
{}into a payload of nothing but nullsBoth assert the payload against
meta.output_schemawithjsonschema.validate, so they pin theactual contract — structuredContent must satisfy the schema this server published — rather than
just a dict shape.
They also call the tool function instead of restating its return value, and add no
# pragma: no cover. That is the point: the existing tests for this shape never invoke the toolbody, which is why the bug survived — and building the annotation for real is what exposed the
3.10 crash as well.
The test module now takes
TypedDictfromtyping_extensions, which pydantic requires below 3.12for a
NotRequiredqualifier to be honoured.Docs
Per
AGENTS.md("update the relevant page(s) underdocs/in the same PR"),docs/servers/structured-output.mdsaid aTypedDict'sstructured_contentwas "identical to theBaseModelversion". That is no longer true for omitted keys, so theTypedDictsection now statesthe omission rule and notes the
typing_extensionsimport requirement below 3.12.Disclosure
Written with AI assistance (Claude Code). Filed by me as the human contributor -- I own this
change and will answer any questions on the implementation or the trade-offs above.