Skip to content

Mapping-set selection and rc.2 x-oold-context transform - #154

Open
simontaurus wants to merge 3 commits into
mainfrom
feat/mapping-set-selection
Open

simontaurus wants to merge 3 commits into
mainfrom
feat/mapping-set-selection

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Changes

  • oold.utils.mappings (new): promote, mapping_sets, chain, context_of, synonym_entries. This is the profile-based selection OOLD-EXT-4966 describes; it complements validation.context_resolution.promoted_terms, which deliberately never chooses a synonym (OOLD-EXT-8f62). Ported from oold-reference-schemas/scripts/{_shared,build_docs}.py and verified equivalent against that implementation on the quantities module (all schemas, all three mapping sets plus consensus). Two deliberate extensions over the port: mapping_set_id may be a list (the spec allows it), and a @reverse synonym promotes to @reverse rather than an invalid @id+@reverse pair.
  • oold.utils.transform (replaced): to_rdf / from_rdf / transform over x-oold-context. Export promotes a named set; import rewrites every exactMatch synonym onto the primary IRI (no set needed), inverts @reverse relations, frames @graph input via validation.frame.schema_to_frame, and bridges document shapes when a promoted fragment carries @nest and a set is named. Readings A/B/C/E of Mapping-set selection can bridge document shapes via @nest, and the spec should say so oold-schema#135 are reproduced exactly and pinned as regression tests, including the boundary that vocabulary is bridged without a selection and shape is not.
  • tests/test_transform.py rewritten: the name* alias cases are re-encoded as x-oold-context with the same inputs and expected outputs, so the change of notation provably does not change what the transformation produces.

Breaking change

jsonld_to_jsonld / json_to_json and the pre-spec name* alias notation are removed. Only tests/test_transform.py referenced them in this repository. The commit is deliberately typed feat: rather than feat!: so semantic-release does not force a major bump; retype if a major is wanted.

Verification

Closes the mappings/transform part of the migration listed in https://github.com/OO-LD/oold-playgrounds/blob/feat/schema-playground/schema-playground/README.md#pending-upstream-migration

- add oold.utils.mappings: promote, mapping_sets, chain, context_of,
  synonym_entries; the selection OOLD-EXT-4966 describes, complementing
  validation's promoted_terms which deliberately never chooses
- replace the name* alias notation in oold.utils.transform with
  to_rdf/from_rdf/transform over x-oold-context: exactMatch rewriting,
  @reverse inversion, @graph framing, @nest shape bridging under a
  named set (OO-LD/oold-schema#135)
- re-encode the alias-notation test cases as x-oold-context, same
  inputs and expected outputs
@github-actions

github-actions Bot commented Sep 12, 2026

Copy link
Copy Markdown
Contributor

Release preview

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

Changelog preview (truncated)
## v1.1.0 (2026-09-20)

### Features

- **utils**: Rewrite synonyms in JSON-LD rather than in RDF
  ([`d57172b`](https://github.com/OO-LD/oold-python/commit/d57172bb87ea58e9e8d031d3a1ba43de2c23b184))

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)
============================================================

⚠️  Performance Regressions:
  ❌ test_complex_graph: 0.0011s → 0.0017s (+55.3%, ratio: 1.55x)

✅ Performance Improvements:
  ✅ test_subclass_inheritance: 0.0543s → 0.0405s (-25.3%, ratio: 0.75x)

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0012s → 0.0012s (+0.7%)
  ➖ test_sqlite_document_store: 0.0013s → 0.0013s (+0.8%)
  ➖ test_local_sparql_store: 0.0289s → 0.0286s (-1.2%)
  ➖ test_oneof_subschema: 0.0438s → 0.0465s (+6.2%)
  ➖ test_enum_docstrings: 0.0402s → 0.0441s (+9.7%)
  ➖ test_class_hierarchy: 0.0372s → 0.0441s (+18.5%)
  ➖ test_core[v1]: 0.0279s → 0.0348s (+24.7%)
  ➖ test_core[v2]: 0.0328s → 0.0384s (+17.2%)
  ➖ test_schema_generation[v1]: 0.0012s → 0.0012s (-0.2%)
  ➖ test_schema_generation[v2]: 0.0020s → 0.0020s (-0.4%)
  ➖ test_simple_json: 0.0004s → 0.0005s (+18.6%)

============================================================
Summary: 1 regressions, 1 improvements, 11 unchanged
============================================================

⚠️  Regressions detected but not failing build (informational only)

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 12, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 84.14634% with 39 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/oold/utils/transform.py 80.9% 15 Missing and 14 partials ⚠️
src/oold/utils/mappings.py 89.3% 4 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

@simontaurus

simontaurus commented Sep 20, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed at 156b9b0 merged with current main (clean merge, 757 pass). The 27 added tests pass, so none of the below is caught by the suite.

Blocking: silent, total data loss. to_rdf never materialises the schema's x-oold-instance-rdf-type as @type, so the frame from_rdf derives matches nothing:

schema = {"@context": CTX, "x-oold-instance-rdf-type": ["ex:Person"], "properties": {...}}
inst   = {"id": "https://ex.org/p1", "name": "Jane", "address": {...}}
transform(inst, [schema], [schema])
# -> {'@context': {...}}        every triple gone, no error

validation.roundtrip.roundtrip() already does this correctly - it sets document["@type"] = instance_rdf_types(schema) when the sample carries none.

The RDF detour is not needed, and the reason is not cost. _apply_rewrites justifies itself with "a single context can only give each term one reading". The implementation being replaced answered that by synthesising one bridge term per synonym and swapping the context under the document - compact under a context where _ex_worksFor means the synonym IRI, redefine that key to the primary IRI, flatten, compact again. It does generalise to x-oold-context: one bridge per synonym, plus one per @type: @vocab property so value positions compact against it.

Measured over the schemas.oo-ld.org cases and synthetic graphs, all three candidates agree on every corpus case and separate only on constructed input:

200 nodes / 800 triples
  as written here (n-quads text round trip) : 133.3 ms
  rewrite pyld's dataset dict in place      :  50.2 ms
  bridge terms, pure JSON-LD                :  60.1 ms

So rewriting the dataset dict is the faster of the two viable options, and it still cannot be made correct. An RDF dataset carries no term definitions, so nothing in it separates a @type: @vocab value - a term drawn from a vocabulary, which a synonym rewrite must follow - from a reference to a node, which it must not touch. A document holding the same IRI in both positions rewrites both or neither. Compaction, expansion and flattening are the normative algorithms; pyld's dataset dict shape is documented nowhere.

Separately, _expand_iri calls jsonld.expand per synonym side: 40 synonyms is 80 processor invocations per from_rdf, where one probe document would do.

Correctness

  • to_rdf turtle branch (the default) serializes an rdflib Dataset with default_union=False, dropping every named-graph quad; format=NQUADS returns the quad, default returns '\n'.
  • promote's elif rest: replaces the whole term definition, discarding the primary's @type/@container - a reference exports as a string literal. The else two lines down preserves coercion deliberately.
  • promote never consults is_exact_match, so a closeMatch tagged with the selected set is promoted and exported as an equivalence - contradicting the module docstring at line 27.
  • from_rdf frames from schemas[-1] alone, so a base-declared x-oold-instance-rdf-type or embedded property is invisible and a nested node surfaces both embedded and as a sibling. schema_to_frame({"allOf": list(reversed(schemas))}, context) works, since instance_rdf_types and collect_composed_properties both walk allOf.
  • _apply_rewrites' invert branch adds (obj, pred, subject) without checking obj is not a Literal; rdflib accepts the literal subject and pyld then fails to parse it, one layer later.
  • null-entry removal is documented but not implemented: synonyms_of drops null entries rather than recording a removal, so a derived schema cannot cancel a base's synonym.
  • chain's cycle guard keys on id(node), which only holds if the resolver returns the identical object per $ref; a parse-per-call resolver recurses without bound. validation.context_resolution guards with an explicit stack and a depth limit.
  • _set_ids / is_exact_match / synonyms_of assume dicts and raise AttributeError on a string or list, against the tolerance chain's docstring claims.
  • _id_keys recognises only the string form of an @id alias, so {"id": {"@id": "@id"}} leaves _:b0 in the output.
  • Unknown format falls through to turtle, and an unrecognised set_id silently yields the consensus reading, though mapping_sets() knows the valid ones.
  • The framing decision and the RDF base disagree with validation.roundtrip, which picks framing from embedded_properties(schema) rather than a node count, and threads a base through every pyld call.

Process

  • Removing the public jsonld_to_jsonld / json_to_json under feat(utils): with no ! cuts a minor release for a breaking removal. Now that main is 1.x this needs feat!: or a BREAKING CHANGE: footer.
  • The module docstring still says from_rdf takes no set_id; the signature has one and the shape-bridging feature depends on it.
  • tests/test_transform.py uses banner comments, and neither new module appears in docs/api.md.

- bridge terms carry the rewrite: compact under a context where the term
  means the synonym IRI, redefine it to the primary IRI, flatten, recompact
- a dataset carries no term definitions, so rewriting RDF cannot tell a
  @vocab-coerced value from a reference to a node
- from_rdf reads a JSON-LD document as it stands instead of sending it
  through RDF and back, keeping @index and @direction
- keep jsonld_to_jsonld and json_to_json, deprecated, with their tests
- Dataset.default_union, else triples() drops every quad in a named graph
@github-actions

Copy link
Copy Markdown
Contributor

📊 Benchmark Results

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

⚠️  Performance Regressions:
  ❌ test_complex_graph: 0.0016s → 0.0032s (+95.2%, ratio: 1.95x)

➖ Unchanged (within threshold):
  ➖ test_simple_dict_document_store: 0.0019s → 0.0020s (+0.7%)
  ➖ test_sqlite_document_store: 0.0021s → 0.0021s (+0.2%)
  ➖ test_local_sparql_store: 0.0379s → 0.0376s (-0.9%)
  ➖ test_oneof_subschema: 0.0598s → 0.0598s (-0.0%)
  ➖ test_enum_docstrings: 0.0503s → 0.0507s (+0.9%)
  ➖ test_subclass_inheritance: 0.0531s → 0.0537s (+1.0%)
  ➖ test_class_hierarchy: 0.0507s → 0.0514s (+1.4%)
  ➖ test_core[v1]: 0.0353s → 0.0357s (+1.2%)
  ➖ test_core[v2]: 0.0428s → 0.0436s (+2.0%)
  ➖ test_schema_generation[v1]: 0.0016s → 0.0016s (+2.0%)
  ➖ test_schema_generation[v2]: 0.0037s → 0.0037s (+0.8%)
  ➖ test_simple_json: 0.0007s → 0.0008s (+17.6%)

============================================================
Summary: 1 regressions, 0 improvements, 12 unchanged
============================================================

⚠️  Regressions detected but not failing build (informational only)

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.

1 participant