Skip to content

fix!: emit schemas that validate against OO-LD v1.0.0-rc.3 - #162

Open
simontaurus wants to merge 7 commits into
mainfrom
phase-a-emission-conformance
Open

simontaurus wants to merge 7 commits into
mainfrom
phase-a-emission-conformance

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Phase A of the reference-implementation plan. oold-python is the OO-LD reference implementation and rc.3's from-python.md points users at model_json_schema() as the way to emit a schema - so what a model emits is the published artifact. It did not validate.

Measured with this repo's own validator against the vendored rc.3, before:

emitter result
model_json_schema() 3 ok, 2 failed
export_schema() 4 ok, 1 failed

Four internal shapes were leaking into the document

  • A link is type: string, not a $ref. A link serialises to an IRI, and a real OSW schema says {"type": "string", "range": "Category:OSW09f6..."}. The $ref/allOf form is what generator.preprocess builds so datamodel-code-generator emits Optional[Bar]. Published, it described a document the library never writes, and the JSON-LD round trip failed on it: with "@type": "@id" an embedded object loses its properties. Union arms keep their union.
  • Requiredness is required alone. x-oold-required-iri and x-oold-link are field annotations - neither is in the rc.3 vocabulary (0 occurrences). The first carries the requirement across the point where code generation must drop the property from required. Mirrored for v1 in static.export_schema, which pydantic v1 reaches without the v2 hook.
  • A required property no longer also carries default: null, which nothing can satisfy.
  • $id is carried up to the document when a self-referential model returns a {"$defs": …, "$ref": …} wrapper.

A partial export emits one schema level

SchemaExportMode.PARTIAL was configuration that did nothing - the same monolithic schema as FULL, every inherited property inlined, no allOf, with or without cutoff_base_cls. _export_schema_from_dynamic_model built its "model itself" copy from model_fields, which pydantic has already flattened.

Now Person.json carries what Person adds and composes onto its base, with @context mirroring allOf in the same order - OOLD-CMP-b926, OOLD-CMP-e4a3, OOLD-CMP-f3c7. Previously the base $ids reached @context only, so the document claimed an inheritance it never declared. This library's own bases are skipped ({"$ref": "LinkedBaseModel"} resolves to nothing); a user's base without an $id keeps being named by its class. Stale unreferenced $defs are pruned. FULL stays the default.

Breaking

A bare Link[T] annotation is now optional (closes #159). Requiredness is always explicit, because it propagates into resolution: reading a link constructs the target, so a required link makes every stored document lacking it unconstructible - a self-referential link like father could never be satisfied by a real dataset.

Verification

742 passed, and 732 under OOLD_DESCRIPTOR_BINDING=0. oold validate tests/data/oold: 380 ok, 0 failed. ruff, ruff format, ty, deptry and the strict docs build all clean. Two new test modules assert the emitted document validates, rather than asserting a shape by hand.

A schema this library emits is the published artifact - rc.3's own
from-python.md points users at model_json_schema() - so three internal
shapes were leaking into it, and it did not validate against the spec we
implement.

- a link serialises to an IRI, so its property is `type: string` (or an
  array of strings). The $ref/allOf form is what generator.preprocess
  builds so datamodel-code-generator emits Optional[Bar]; published, it
  described a document the library never writes, and the JSON-LD round
  trip failed on it - with "@type": "@id" an embedded object loses its
  properties. Union arms keep their union: they genuinely accept a
  literal, a reference or an inline object
- requiredness is stated by `required` alone; x-oold-required-iri and
  x-oold-link are field annotations and stay internal. Mirrored for v1
  in static.export_schema, which pydantic v1 reaches without the v2 hook
- a required property no longer also carries `default: null`, which
  nothing can satisfy
- carry $id up to the document when a self-referential model returns a
  {"$defs": ..., "$ref": ...} wrapper

The legacy binding has no emission hook and is skipped: it is the
deprecated opt-out, not what publishes.
Closes #159. All three spellings below are optional to supply; only the
explicit argument makes a link required:

    father: Link["Person"]
    father: Link["Person"] = OoldField()
    father: Link["Person"] = OoldField(required=True)

"No default means required" reads well in plain Python but is wrong for
a link, because requiredness propagates into resolution: resolving a
link constructs the target, so a required link makes every stored
document lacking it unconstructible - and a self-referential link like
father could never be satisfied by a real dataset. Links are declared
far more often than they are required, so the terse form is the common
case.

The bare form still gets an injected OoldField() for its default=None; a
link cannot be required at the pydantic level, since its value never
reaches validation.
The schema hierarchy now mirrors the class hierarchy. PARTIAL was
configuration that did nothing: it emitted the same monolithic schema as
FULL, with every inherited property inlined and no allOf, whether or not
cutoff_base_cls was given.

- _export_schema_from_dynamic_model built its "model itself" copy from
  model_fields, which pydantic has already flattened to include inherited
  fields. A level is the difference against its bases; restating an
  inherited property can also relax it, which OOLD-CMP-f3c7 forbids
- emit allOf for each composable base. The base $ids were already
  collected and reached @context only, so the document claimed an
  inheritance it never declared - the inverse of OOLD-CMP-b926, and
  OOLD-CMP-e4a3 wants the two in the same order
- skip this library's own bases: {"$ref": "LinkedBaseModel"} resolves to
  nothing. A user's base without an $id keeps being named by its class
- drop $defs entries nothing references: the dynamic copy left a stale,
  flattened definition behind that contradicted the level beside it

FULL stays the default and keeps its meaning.
- requiredness reaches the schema as `required` alone;
  x-oold-required-iri is a field annotation and stays internal
- a bare Link[T] annotation is optional, not required
- a link property is `type: string` (or an array of strings), not a $ref
  to the target; the $ref form belongs to code generation
- note the @context terms a link needs, including @container on a
  strictly array-typed property
@github-actions

github-actions Bot commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v2.0.0 (current: v1.0.1).

Changelog preview (truncated)
## v2.0.0 (2026-09-19)

### Bug Fixes

- A partial export emits one schema level, composed with allOf
  ([`076defe`](https://github.com/OO-LD/oold-python/commit/076defe5d3b48c92100aafb7bc5b8af49c881ee2))

- Declare an IRI-family format on emitted link properties
  ([`75c7c86`](https://github.com/OO-LD/oold-python/commit/75c7c8641bd7569847beca2cda6d50cb33d78dee))

- Derive x-oold-range from the target's location, not its identity
  ([`2e51a25`](https://github.com/OO-LD/oold-python/commit/2e51a2598645e3bf7192757d89c095de76def42d))

- Publish the document shape, not the code-generation shape
  ([`f6739fe`](https://github.com/OO-LD/oold-python/commit/f6739fe75073fdf28e7805191f839921941cdeec))

### Documentation

- Correct what a schema says about a link
  ([`77f7d85`](https://github.com/OO-LD/oold-python/commit/77f7d852fa26f42b11401123ed6c078979a01321))

### Features

- A bare Link[T] annotation is optional
  ([`1eb6a57`](https://github.com/OO-LD/oold-python/commit/1eb6a57506ae36fec9b6bbdd52a2b0fa3c5dc879))

### Testing

- Cover def pruning and the v1 requiredness spelling
  ([`95bd561`](https://github.com/OO-LD/oold-python/commit/95bd5618b0830114101352d919e442ca0187ccab))

Preview via python-semantic-release and conventional commits.

@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0019s → 0.0020s (+1.4%)
  ➖ test_sqlite_document_store: 0.0021s → 0.0021s (+0.6%)
  ➖ test_local_sparql_store: 0.0385s → 0.0380s (-1.3%)
  ➖ test_oneof_subschema: 0.0612s → 0.0615s (+0.4%)
  ➖ test_enum_docstrings: 0.0512s → 0.0526s (+2.6%)
  ➖ test_subclass_inheritance: 0.0547s → 0.0552s (+0.9%)
  ➖ test_class_hierarchy: 0.0516s → 0.0529s (+2.5%)
  ➖ test_core[v1]: 0.0354s → 0.0368s (+3.8%)
  ➖ test_core[v2]: 0.0440s → 0.0458s (+4.0%)
  ➖ test_schema_generation[v1]: 0.0016s → 0.0017s (+3.5%)
  ➖ test_schema_generation[v2]: 0.0037s → 0.0038s (+1.7%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+0.9%)
  ➖ test_complex_graph: 0.0016s → 0.0017s (+3.0%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

@codecov

codecov Bot commented Sep 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.16667% with 19 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/oold/static.py 84.2% 5 Missing and 7 partials ⚠️
src/oold/model/_descriptor.py 84.0% 5 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0019s → 0.0020s (+1.7%)
  ➖ test_sqlite_document_store: 0.0022s → 0.0022s (-3.0%)
  ➖ test_local_sparql_store: 0.0425s → 0.0411s (-3.3%)
  ➖ test_oneof_subschema: 0.0679s → 0.0679s (+0.0%)
  ➖ test_enum_docstrings: 0.0572s → 0.0575s (+0.5%)
  ➖ test_subclass_inheritance: 0.0606s → 0.0615s (+1.5%)
  ➖ test_class_hierarchy: 0.0569s → 0.0587s (+3.1%)
  ➖ test_core[v1]: 0.0392s → 0.0401s (+2.5%)
  ➖ test_core[v2]: 0.0479s → 0.0494s (+3.2%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0018s (+4.8%)
  ➖ test_schema_generation[v2]: 0.0040s → 0.0041s (+2.0%)
  ➖ test_simple_json: 0.0007s → 0.0008s (+4.7%)
  ➖ test_complex_graph: 0.0017s → 0.0017s (+0.6%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

@simontaurus
simontaurus force-pushed the phase-a-emission-conformance branch from ede0810 to 95bd561 Compare September 19, 2026 17:50
@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0014s → 0.0014s (+1.2%)
  ➖ test_sqlite_document_store: 0.0015s → 0.0015s (+1.4%)
  ➖ test_local_sparql_store: 0.0282s → 0.0304s (+7.6%)
  ➖ test_oneof_subschema: 0.0466s → 0.0543s (+16.5%)
  ➖ test_enum_docstrings: 0.0391s → 0.0401s (+2.8%)
  ➖ test_subclass_inheritance: 0.0401s → 0.0412s (+2.7%)
  ➖ test_class_hierarchy: 0.0400s → 0.0397s (-0.7%)
  ➖ test_core[v1]: 0.0266s → 0.0271s (+1.8%)
  ➖ test_core[v2]: 0.0316s → 0.0381s (+20.5%)
  ➖ test_schema_generation[v1]: 0.0011s → 0.0012s (+5.3%)
  ➖ test_schema_generation[v2]: 0.0027s → 0.0028s (+2.8%)
  ➖ test_simple_json: 0.0004s → 0.0004s (+1.2%)
  ➖ test_complex_graph: 0.0011s → 0.0011s (-0.4%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

OOLD-EXT-6ea3 (SHOULD) wants an IRI-valued property to constrain its
lexical form, and OOLD-EXT-1f92 recommends iri-reference - it admits
absolute IRIs, compact IRIs and context-relative references alike, which
is what instances carry. Our own validator warned on our own output:
"IRI reference properties without an iri-reference/uri* format".

It is also the second of the three reference signals a frame derivation
looks for (OOLD-EXT-68fa), so this keeps the schema side and
oold.validation.frame.reference_properties in agreement - the same
principle #161 applied to framing.

A format the declaration already states is left alone; OSW declares
`format: autocomplete` on link properties for its UI.
@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0019s → 0.0020s (+0.5%)
  ➖ test_sqlite_document_store: 0.0021s → 0.0021s (-0.0%)
  ➖ test_local_sparql_store: 0.0381s → 0.0383s (+0.5%)
  ➖ test_oneof_subschema: 0.0604s → 0.0631s (+4.4%)
  ➖ test_enum_docstrings: 0.0520s → 0.0530s (+1.9%)
  ➖ test_subclass_inheritance: 0.0547s → 0.0547s (+0.0%)
  ➖ test_class_hierarchy: 0.0515s → 0.0532s (+3.3%)
  ➖ test_core[v1]: 0.0357s → 0.0374s (+4.8%)
  ➖ test_core[v2]: 0.0440s → 0.0447s (+1.4%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0017s (+2.5%)
  ➖ test_schema_generation[v2]: 0.0037s → 0.0038s (+1.1%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+4.1%)
  ➖ test_complex_graph: 0.0016s → 0.0016s (+1.2%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

x-oold-range is dereferenced - code generation fetches the target
schema, a form editor renders the targets a property allows - so it must
be where the schema lives.

get_cls_iri() answers identity: it merges the $id with the type field's
default(s), which are the instances' rdf:type. Deriving the range from
it published identities with nothing to fetch at them.
wiki_data.Person answered ["http://www.wikidata.org/entity/Q5",
"Item:Q5"] and publishes no schema at either, so the emitted range
pointed at a Wikidata class.

The range now comes from $id alone. A class that does not say where its
schema lives contributes none; the property is still marked a reference
by its format, the second signal in OOLD-EXT-68fa. Where location and
identity coincide nothing changes.
@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

Click to see benchmark comparison
📊 Benchmark Comparison (threshold: 1.3x)
============================================================

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0020s → 0.0019s (-4.9%)
  ➖ test_sqlite_document_store: 0.0022s → 0.0021s (-5.5%)
  ➖ test_local_sparql_store: 0.0378s → 0.0412s (+8.9%)
  ➖ test_oneof_subschema: 0.0605s → 0.0640s (+5.8%)
  ➖ test_enum_docstrings: 0.0511s → 0.0563s (+10.2%)
  ➖ test_subclass_inheritance: 0.0541s → 0.0594s (+9.8%)
  ➖ test_class_hierarchy: 0.0519s → 0.0555s (+6.8%)
  ➖ test_core[v1]: 0.0350s → 0.0380s (+8.8%)
  ➖ test_core[v2]: 0.0444s → 0.0446s (+0.5%)
  ➖ test_schema_generation[v1]: 0.0016s → 0.0017s (+4.7%)
  ➖ test_schema_generation[v2]: 0.0037s → 0.0038s (+3.0%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+0.3%)
  ➖ test_complex_graph: 0.0016s → 0.0016s (+0.2%)

============================================================
Summary: 0 regressions, 0 improvements, 13 unchanged
============================================================

✅ No significant performance regressions

Threshold: 1.3x (30% slower triggers a regression warning)

Note: Benchmarks are informational only and won't fail the build.

💡 Tip: Download the benchmark-results artifact for detailed JSON data

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should a bare Link[T] annotation mean required, or optional?

1 participant