Skip to content

fix(validation): count a scoped @context term only where it is a property - #135

Merged
simontaurus merged 2 commits into
mainfrom
fix/scoped-term-not-a-property
Aug 27, 2026
Merged

fix(validation): count a scoped @context term only where it is a property#135
simontaurus merged 2 commits into
mainfrom
fix/scoped-term-not-a-property

Conversation

@simontaurus

Copy link
Copy Markdown
Contributor

Fixes OO-LD/oold-schema#149. Repro from the issue now passes: 30 ok, 0 failed, 0 warning(s).

Cause

embedded_properties (src/oold/validation/frame.py) took every @context term carrying a scoped @context, without checking that the schema declares a property of that name.

OOLD-CMP-b926 requires a schema to reflect every $ref in its @context, including $refs reached from $defs. In the repro, amount lives in $defs/Component, so its term is in the root context by that rule, but the root object has no amount property and no instance of it can. Counting the term had two effects, both from the same call:

  1. roundtrip() chose framed over compacted
  2. schema_to_frame() emitted "amount": {}

A frame property is a match filter, so the frame selected nodes carrying amount. The generated instance {"type": ["Composition"]} carries none, framing returned nothing, and type was reported lost. type was never involved; it was just the only property present.

This is why the two branches in the issue looked contradictory: removing the scoped @context removed the phantom embed, so the round-trip fell back to compaction and passed, while OOLD-CMP-5266 returned. The rule and the check never actually disagreed.

Fix

Require a scoped term to name a property the schema declares. schema here is dereferenced, so collect_composed_properties sees properties inherited through allOf as well.

No spec change: OOLD-CMP-5266 recommends exactly what the round-trip supports, and the three warnings in the materials schemas can now be acted on.

Verification

  • repro from the issue: FAIL -> PASS
  • pytest: 558 passed, 9 skipped, plus a regression test for the $defs case
  • oold-schema/examples: 380 ok, 0 failed, unchanged warnings
  • parity against validate.mjs: 6 passed

scripts/schema_to_frame.mjs in oold-schema has the same defect; it is not exercised by the parity fixtures. Fixed there in OO-LD/oold-schema#151 so the reference implementation carries it into oold-js.

…erty

A schema must reflect every $ref in its @context (OOLD-CMP-b926), including
the ones reached from $defs, so the root context carries terms for properties
the root object never holds. embedded_properties counted every scoped term,
which put an unmatchable property into the derived frame and made framing
return nothing.

Closes OO-LD/oold-schema#149
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Release preview

Merging this PR would release v0.18.1 (current: v0.18.0).

Changelog preview (truncated)
## v0.18.1 (2026-08-27)

### Bug Fixes

- **validation**: Count a scoped @context term only where it is a property
  ([`4f4c39c`](https://github.com/OO-LD/oold-python/commit/4f4c39c0723a423d49df54ff742abb389d239f43))

### Code Style

- Apply ruff format to the new test
  ([`c23383d`](https://github.com/OO-LD/oold-python/commit/c23383dad981c03c46960a362094090b333831c2))

### Continuous Integration

- Gate the parity check against the reference
  ([`fdb9007`](https://github.com/OO-LD/oold-python/commit/fdb9007377aab115da3b2661e59834090af79314))

### Documentation

- **validation**: State invariants in comments, not prior behaviour
  ([`4baf212`](https://github.com/OO-LD/oold-python/commit/4baf212f85a6f97fc9f6f16eee07ab2a0bcd42d5))

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.0017s → 0.0017s (+0.5%)
  ➖ test_sqlite_document_store: 0.0019s → 0.0019s (+0.3%)
  ➖ test_local_sparql_store: 0.0380s → 0.0389s (+2.3%)
  ➖ test_oneof_subschema: 0.0589s → 0.0587s (-0.4%)
  ➖ test_enum_docstrings: 0.0494s → 0.0496s (+0.4%)
  ➖ test_subclass_inheritance: 0.0528s → 0.0530s (+0.4%)
  ➖ test_class_hierarchy: 0.0502s → 0.0507s (+0.9%)
  ➖ test_core[v1]: 0.0376s → 0.0384s (+2.1%)
  ➖ test_core[v2]: 0.0433s → 0.0452s (+4.4%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0017s (+0.1%)
  ➖ test_schema_generation[v2]: 0.0028s → 0.0028s (+0.7%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+0.7%)
  ➖ test_complex_graph: 0.0016s → 0.0016s (+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 Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 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.0019s (-0.7%)
  ➖ test_sqlite_document_store: 0.0020s → 0.0019s (-6.3%)
  ➖ test_local_sparql_store: 0.0450s → 0.0429s (-4.7%)
  ➖ test_oneof_subschema: 0.0671s → 0.0657s (-2.1%)
  ➖ test_enum_docstrings: 0.0561s → 0.0560s (-0.1%)
  ➖ test_subclass_inheritance: 0.0619s → 0.0607s (-2.1%)
  ➖ test_class_hierarchy: 0.0567s → 0.0587s (+3.5%)
  ➖ test_core[v1]: 0.0421s → 0.0414s (-1.6%)
  ➖ test_core[v2]: 0.0496s → 0.0488s (-1.5%)
  ➖ test_schema_generation[v1]: 0.0017s → 0.0017s (-2.2%)
  ➖ test_schema_generation[v2]: 0.0029s → 0.0029s (-2.0%)
  ➖ test_simple_json: 0.0007s → 0.0007s (+1.5%)
  ➖ test_complex_graph: 0.0016s → 0.0017s (+1.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 0b49605 into main Aug 27, 2026
21 checks passed
@simontaurus
simontaurus deleted the fix/scoped-term-not-a-property branch August 27, 2026 03:31
simontaurus added a commit to OO-LD/oold-schema that referenced this pull request Aug 27, 2026
#151)

Mirrors OO-LD/oold-python#135 so the reference implementation and oold-python
stay in parity, and so the fix travels with validate.mjs into oold-js.

Refs #149
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.

Property-scoped @context makes roundtrip.generated drop an unrelated property

1 participant