Repository navigation
Python: Exclude ClassVar annotations from generated JSON schemas - #14528
Pushpak Siva Sai (Pushpak731) wants to merge 2 commits into
Conversation
KernelJsonSchemaBuilder.build_model_schema builds the instance schema
from get_type_hints, which includes ClassVar annotations, so a class
constant became a required object-valued property. Pydantic excludes
ClassVars from model fields and serialized instances, so the generated
schema rejected valid payloads (a serialized {'value': 1} failed
validation because 'marker' was required). The builder now skips
ClassVar-annotated hints (both the parameterized and bare forms,
including inherited ones), for KernelBaseModel subclasses, plain
annotated classes and instances, covering KernelParameterMetadata,
generated tool definitions and structured_output schemas.
Fixes microsoft#14527
|
@microsoft-github-policy-service agree |
PRABHU KIRAN VANDRANKI (VANDRANKI)
left a comment
There was a problem hiding this comment.
Community review. This does not clear the merge gate, it is one reader's check.
I reproduced the bug and checked the fix by running code. I loaded the installed semantic_kernel/schema/kernel_json_schema_builder.py (no ClassVar handling in it), then built a copy with your two added lines applied by string patch, and compared KernelJsonSchemaBuilder.build output for three classes.
class M(KernelBaseModel): marker: ClassVar[str] = "c"; value: int- current code: properties
marker: {"type": "object"}andvalue, required["marker", "value"] - with the patch: only
value, required["value"] M(value=1).model_dump()is{"value": 1}andM.model_fieldsis["value"], so the current schema demands a field the model never serializes, and it also types it wrongly asobject.
- current code: properties
Annotated[ClassVar[int], "x"]on a KernelBaseModel: same result, the patch drops it.- A plain (non-pydantic) class with a string annotation
"ClassVar[dict]": also dropped, becauseget_type_hintsresolves the string before the check.
The check field_type is ClassVar or get_origin(field_type) is ClassVar covers both the bare ClassVar and the subscripted form, which are the two shapes get_type_hints returns. It also fits with the loop: nothing after the continue depends on the skipped field, so required and properties stay consistent.
Notes:
- I did not run the repo's test file or the full suite, only the builder on the three classes above. The new tests (inherited ClassVar, structured output with
additionalProperties, via-instance) read correctly to me, but I did not execute them. - I did not check
typing_extensions.ClassVaron older Pythons. It is the same object astyping.ClassVaron current versions, so I expect it to work, but that is unverified. - Nit: the ten new tests overlap a lot. Two or three would cover the same branches (bare, subscripted, inherited).
Approving from my side.
Fixes #14527
Description
KernelJsonSchemaBuilder.build_model_schemabuilds the schema fromget_type_hints, which includesClassVarannotations, so a class constant became a required object-valued property in the generated schema. Pydantic excludesClassVars frommodel_fieldsand from serialized instances, so the resulting schema rejected valid serialized payloads ({'value': 1}failed becausemarkerwas required). The same wrong schema reachedKernelParameterMetadata, generated function-call tool definitions andstructured_output=Trueschemas.The fix skips
ClassVar-annotated hints before the required/property loop — covering the parameterized form (ClassVar[str]), the bare form (ClassVar, whoseget_originisNone), and inheritedClassVars, forKernelBaseModelsubclasses, plain annotated classes and model instances alike.Contribution Checklist
tests/unit/schema/(45 passed) andtests/unit/functions/test_kernel_parameter_metadata.py(7 passed) — 10 new regression tests cover pydantic models, bare/inheritedClassVar, plain classes, instances,structured_output, descriptions, optionals and the serialized-payload contractruff checkandruff format --checkare clean on both changed filesmodel_dump)