Skip to content

Delegate JSON Schema parsing to a library; keep only Stickler-specific semantics #212

Description

@sromoam

Summary

from_json_schema() hand-rolls JSON Schema parsing. That parser is incomplete, and we have been completing it one spelling at a time, one bug report at a time. We should delegate the commodity half of the work to a real JSON Schema library and keep only the half that is genuinely ours.

This is an architecture issue, not a bug. Nothing here blocks the fixes currently in flight, and I am explicitly not proposing we hold them.

The evidence

Every one of these is the same underlying problem surfacing in a different spelling:

scope
#105 from_json_schema() fails on valid nullable schemas (anyOf, type: [X, null], implicit objects)
#127 (merged) added type: ["string", "null"]
#198 adds anyOf [X, null] and implicit objects; still does not handle oneOf
#159 optional fields need Optional[T] annotations so None defaults validate
#146 PEP 604 X | None, the Python-side mirror of the same gap
#161 _unwrap_optional is PEP-604-blind, so schema export emits {"type": "string"} for X | None
#162 HTML report thresholds miss optional/union-wrapped nested models
#163 (merged as #183) removed a duplicate type classifier that existed because of this drift

Eight items. Every one is "we did not handle a spelling the spec allows." That is the signature of a hand-rolled parser: coverage grows by bug report rather than by conformance.

Two things make it concrete:

1. We break on Pydantic's own output. anyOf [X, {"type": "null"}] is exactly what Pydantic v2 emits for every Optional field:

class P(BaseModel):
    name: Optional[str] = None
    addr: Optional[Addr] = None

P.model_json_schema()["properties"]
# name -> {"anyOf": [{"type": "string"}, {"type": "null"}]}
# addr -> {"anyOf": [{"$ref": "#/$defs/Addr"}, {"type": "null"}]}

So before #198, from_json_schema(SomeModel.model_json_schema()) raised on any model with an optional field. We were failing to ingest the output of the library we are built on.

2. Two in-flight PRs now conflict in this file. #159 and #198 both rewrite convert_property_to_field, and #198 does not apply on top of #159. That is a coordination cost created by having all schema semantics funnel through one hand-written dispatcher.

What is actually ours, and what is not

json_schema_field_converter.py is 660 lines doing two unrelated jobs:

Job 1 — JSON Schema to Python types. $ref resolution, anyOf/oneOf/allOf, nullable spellings, implicit objects, array items, required/default handling. This is a commodity, solved problem, and it is where all eight items above live.

Job 2 — x-aws-stickler-* to comparison semantics. Mapping extensions to comparator, threshold, weight, and clip_under_threshold, then emitting ComparableField() so compare_with() can score:

invoice_id  comparator=ExactComparator        threshold=0.5  weight=1.0
notes       comparator=LevenshteinComparator  threshold=0.85 weight=2.0

This is genuinely ours and not replaceable. No off-the-shelf importer produces it; it is the point of the library.

To be clear about one thing, since it is the obvious first question: Pydantic cannot do Job 1 for us. Its JSON Schema support is deliberately one-directional.

BaseModel.model_json_schema()   exists=True    (model -> schema)
BaseModel.from_json_schema      exists=False   (schema -> model)
BaseModel.parse_json_schema     exists=False

So from_json_schema() is not duplicating Pydantic. It is filling a gap Pydantic left open on purpose. The problem is not that the layer exists; it is that we wrote a schema parser inside it.

Proposal

Keep from_json_schema() as the public API. Change its internals to delegate Job 1 and keep Job 2:

schema --[library]--> plain BaseModel --[our code]--> StructuredModel + ComparableField

The second arrow already largely exists as StructuredModel.from_pydantic(), built for the 0.6.0 zero-config work. That is what makes this tractable rather than a rewrite.

Expected outcome: most of the 660 lines of type dispatch goes away, oneOf/allOf/$ref edge cases come for free, and the class of bug represented by the eight items above stops recurring.

Open questions to answer before committing

This needs a real evaluation, not just a dependency swap:

  1. Does a candidate library preserve unknown keywords? If it drops x-aws-stickler-*, Job 2 has nothing to read and the whole plan fails. This is the make-or-break question. Related: [BUG]: unrecognized x-aws-stickler-* keys are silently dropped, and explain() reports the fallback as "explicit" #210, where unrecognized extension keys are already silently dropped.
  2. Coverage. Does it handle oneOf, allOf, $ref (including remote and recursive), patternProperties, and the nullable spellings we care about?
  3. Dependency weight. We just spent [CHORE]: slim the core dependencies, drop dead psutil, and lower the Python floor to 3.10 #201/Changes for #201. #207 getting import stickler down to 422 modules with six core dependencies. A schema library must not undo that, so it likely belongs behind an extra or must be genuinely light.
  4. Behavioral parity. Does anything currently accepted start being rejected, or vice versa? This is a public API with users, so we need a difference report across a corpus of real schemas, not spot checks.
  5. Candidate comparison. [BUG]: StructuredModel.from_json_schema() fails on valid nullable JSON Schema fields #105 mentions json-schema-to-pydantic. There are others. Someone should compare at least two against the above.

Explicitly out of scope

Filed for 0.8.0 rather than 0.7.0 deliberately: doing this while several PRs are open against the same file would be the wrong order.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    Status
    Backlog

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions