Skip to content

fix(frame): keep reference-valued properties as IRIs - #161

Merged
simontaurus merged 1 commit into
mainfrom
fix/frame-reference-embed-never
Sep 19, 2026
Merged

simontaurus merged 1 commit into
mainfrom
fix/frame-reference-embed-never

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Port of OO-LD/oold-js#3; same defect, since this module is a port of schema_to_frame.mjs.

schema_to_frame emitted no subframe for reference-valued properties, on the assumption stated in the module docstring: a referenced IRI with no local triples stays {"id": ...}. Where the target does carry triples in the same graph, framing pulls them in as an object and the framed document stops validating against the schema the frame was derived from, which declares a string there.

OOLD-EXT-68fa already requires @embed: @never here, and the worked example in the specification's #framing section already prints it, so both implementations were in breach of an existing MUST.

Reference signals: x-oold-range, an IRI-family format (the family OOLD-EXT-6ea3 recommends), or a term mapped "@type": "@id". Embedding wins where a property carries both.

keyword_alias_keys

Thing.schema.json declares id with "format": "iri" while its context aliases id to @id, so the reference signals match it. A subframe there writes {"@id": {...}} and pyld fails with "@id" value must be a string. Keys aliasing a JSON-LD keyword are excluded.

The alias is searched across the composed schema: after dereferencing, Contact keeps Thing's @context on its allOf member rather than at the root, so a root-only scan misses it. This surfaced from the committed corpus, not from a unit test, and the same fix went back into oold-js.

Verified

502 passed, 7 skipped across tests/test_validation/. Removing only the @never line fails exactly the two new framing tests, so they are not vacuous.

@github-actions

github-actions Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Release preview

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

Changelog preview (truncated)
## v1.0.1 (2026-09-19)

### Bug Fixes

- **frame**: Keep reference-valued properties as IRIs
  ([`0e04a1a`](https://github.com/OO-LD/oold-python/commit/0e04a1a87b3e3db077a3f76d7252e56ee6cbf621))

### Chores

- Classify as Beta
  ([`c147a05`](https://github.com/OO-LD/oold-python/commit/c147a05f7665f1ede18f6b48c99a5c0540640d9a))

### Documentation

- **examples**: Optional children extension in wiki_data
  ([`2f372c3`](https://github.com/OO-LD/oold-python/commit/2f372c3f060fa25770b6194642db4eeca905357c))

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.0018s → 0.0017s (-1.3%)
  ➖ test_sqlite_document_store: 0.0019s → 0.0019s (-1.2%)
  ➖ test_local_sparql_store: 0.0319s → 0.0327s (+2.4%)
  ➖ test_oneof_subschema: 0.0541s → 0.0536s (-0.9%)
  ➖ test_enum_docstrings: 0.0459s → 0.0458s (-0.4%)
  ➖ test_subclass_inheritance: 0.0489s → 0.0504s (+3.0%)
  ➖ test_class_hierarchy: 0.0464s → 0.0461s (-0.6%)
  ➖ test_core[v1]: 0.0311s → 0.0312s (+0.2%)
  ➖ test_core[v2]: 0.0390s → 0.0387s (-0.6%)
  ➖ test_schema_generation[v1]: 0.0015s → 0.0015s (+0.4%)
  ➖ test_schema_generation[v2]: 0.0033s → 0.0034s (+2.2%)
  ➖ test_simple_json: 0.0006s → 0.0006s (-0.6%)
  ➖ test_complex_graph: 0.0014s → 0.0014s (+0.7%)

============================================================
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 86.04651% with 6 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/oold/validation/frame.py 86.0% 3 Missing and 3 partials ⚠️

📢 Thoughts on this report? Let us know!

schema_to_frame emitted no subframe for reference-valued properties, on the
assumption that a referenced IRI carries no local triples. Where the target
does carry triples in the same graph, framing embedded it as an object and the
framed document stopped validating against the schema the frame came from.

Reference signals per OOLD-EXT-68fa: x-oold-range, an IRI-family format, or a
term mapped "@type": "@id". Embedding wins where both appear.

keyword_alias_keys excludes keys aliasing a JSON-LD keyword: id carries an IRI
format and so matches, but a subframe there writes {"@id": {...}}, which pyld
rejects. Found through allOf, since the base schema declares the convention.

Refs OO-LD/oold-schema#160
@simontaurus
simontaurus force-pushed the fix/frame-reference-embed-never branch from 5fd1cbe to 0e04a1a Compare September 19, 2026 13:46
@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.0020s (-1.1%)
  ➖ test_sqlite_document_store: 0.0022s → 0.0021s (-1.2%)
  ➖ test_local_sparql_store: 0.0413s → 0.0407s (-1.7%)
  ➖ test_oneof_subschema: 0.0648s → 0.0651s (+0.5%)
  ➖ test_enum_docstrings: 0.0552s → 0.0557s (+0.8%)
  ➖ test_subclass_inheritance: 0.0587s → 0.0607s (+3.4%)
  ➖ test_class_hierarchy: 0.0564s → 0.0568s (+0.8%)
  ➖ test_core[v1]: 0.0392s → 0.0400s (+1.9%)
  ➖ test_core[v2]: 0.0478s → 0.0509s (+6.6%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0017s (+1.7%)
  ➖ test_schema_generation[v2]: 0.0038s → 0.0038s (-1.3%)
  ➖ test_simple_json: 0.0007s → 0.0007s (-5.4%)
  ➖ test_complex_graph: 0.0017s → 0.0016s (-6.1%)

============================================================
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 merged commit c8297dc into main Sep 19, 2026
21 checks passed
@simontaurus
simontaurus deleted the fix/frame-reference-embed-never branch September 19, 2026 13:51
simontaurus added a commit that referenced this pull request Sep 19, 2026
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.
simontaurus added a commit that referenced this pull request Sep 27, 2026
* fix: publish the document shape, not the code-generation shape

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.

* feat!: a bare Link[T] annotation is optional

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.

* fix: a partial export emits one schema level, composed with allOf

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.

* docs: correct what a schema says about a link

- 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

* test: cover def pruning and the v1 requiredness spelling

* fix: declare an IRI-family format on emitted link properties

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.

* fix: derive x-oold-range from the target's location, not its identity

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.
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.

1 participant