fix(ontology): rebuild stale class URIs in OWL/R2RML/Digital Twin generation and persist Constraints tab - #147
Conversation
…eration and persist Constraints tab OntologyGenerator._resolve_uri, R2RMLGenerator and DigitalTwin graph construction all reconstruct a class/property URI from its local name against the domain's current Base URI when the stored URI does not start with it, instead of using the stale URI as-is. This is the same re-basing problem fixed in the reasoning engines (see the companion PR), applied here to: axioms saved from the Expressions & Axioms editor (an axiom referencing a stale class URI no longer matched the class's own owl:Class declaration in the exported OWL, leaving it effectively orphaned); R2RML generation (mapped instances got an rdf:type disconnected from the real class, and their data-property lookup used the wrong URI too); and the Digital Twin graph (instances of a re-based class did not attach to it). R2RMLGenerator additionally reorders TriplesMap generation so the class comment and logical table are built after the class URI is resolved, and looks up data properties using the resolved URI. Adds two endpoints, POST /ontology/constraints/save and POST /ontology/constraints/delete, backing the Designer's Constraints tab (entity disjointWith/equivalentTo; relationship cardinality, functional/inverse-functional/symmetric/transitive). Previously the tab had no endpoint to persist to, so changes made there were never saved. ReasoningService adds _get_constraints(), reading domain.constraints — where the Constraints tab actually stores its data — so a relationship marked Transitive or Symmetric there is picked up by the graph-reasoning pass. Previously graph reasoning only looked at each property's internal characteristics field, which the Constraints tab never wrote to, so marking a relationship Transitive/Symmetric there had no real effect on reasoning even though the UI accepted the setting.
|
|
benoitcayladbx
left a comment
There was a problem hiding this comment.
Review (no merge)
Thanks for carrying the stale-URI work from the reasoning engines into OWL / R2RML / Digital Twin, and for finally wiring the Constraints tab to a backend. The diagnosis (stored uri is not source of truth; domain.constraints is what the Designer actually writes) is right.
This is not merge-ready yet: the rebase logic is copied three times with three different algorithms, the R2RML lookup change can drop data properties, the new routes skip existing domain validation, and there are no tests / no changelog. Also CLA is still pending on the PR.
Please treat this as one shared helper + tests, not three local if startswith blocks. Companion PRs #146 and #148 should call the same helper so hash vs slash does not drift again.
Must-fix
- Single rebase helper — Extract Function into
URIHelpers(already hasextract_local_name/is_uri). One policy for: currentbase_uri+ local name, with the same separator the caller already uses (OntologyGeneratordoesbase_uri + name;R2RMLGenerator.__init__forces a trailing/). Forcing#in R2RML/Digital Twin after that init is inconsistent with OWL (URIRef(self.base_uri + class_name)). Prefixstartswith(domain_root)is also too weak (http://ex.org/ontomatcheshttp://ex.org/ontology/...). - R2RML
data_prop_uri_lookup— lookup is still keyed by the stored classuri(_build_data_property_uri_lookup). Switching the get toresolved_class_urimisses after a rebase. Look up original and resolved, or rebase the lookup keys too. - Tests — required for new
src/behaviour (.cursor/08,src/.coding_rules.md§10). Minimum:- OWL: axiom/class URI after Base URI change (
tests/units/ontology/test_owl_generator.py) - R2RML:
rdf:type+ attribute predicates still found after rebase (tests/units/mapping/test_r2rml_generator.py) - DT: instance attaches to rebased class
POST /ontology/constraints/save|deleteround-trip +Ontology.validate_constraintrejection (tests/units/api/test_routes.py)_find_properties_by_characteristicreadsdomain.constraints(transitive/symmetric)
- OWL: axiom/class URI after Base URI change (
- Changelog —
changelogs/v0.8.0/<github-user>_YYYY-MM-DD.log(English). CI will block without it. - CLA — still pending on this PR.
Should-fix
- Routes: list mutate +
domain.save()belongs onOntology(.cursor/07— thin routes). Call existingOntology.validate_constraint(and raiseValidationError) before persist. Do not append whenindexis out of range — docstring says only-1appends;999currently appends. - Missing-property fallback in
_find_properties_by_characteristicshould log and skip, not invent a URI from a stale name (wrong transitive closure). - Digital Twin still excludes via unrebased
class_uri(if class_uri in excluded_class_uris).
Session-data
| field | used? | derivable? | recommendation |
|---|---|---|---|
ontology.constraints (DomainSession.constraints) |
yes (Designer + this PR) | no | keep; do not also write properties[].characteristics unless you migrate in one shot |
Dual storage (characteristics vs constraints) is the real design smell. Short term, reading both is fine. Longer term, one write path.
Docs
No Sphinx/README change strictly required for this bugfix. Changelog is the doc gate.
Review summary
- Rule violations: 5 (fixed: 0, plan: 5 — tests, changelog, thin routes +
validate_constraint, Extract Function URI rebase, English-only nits) - Duplication clusters: 1 (stale-URI rebase in OWL / R2RML / Digital Twin / reasoning engines)
- Dead code candidates: 1 (
return URIRef(ref)after failed local-name extract in_resolve_uri) - Session fields removed: 0
- Tests: not run on this branch (dirty local worktree; PR head not checked out). PR itself adds 0 tests.
- Docs updated: no (changelog missing)
| # Stale class URI from a previous base_uri — rebuild using '#', | ||
| # matching how classes are declared in the OWL ontology. | ||
| local = self._extract_local_name(class_uri) | ||
| resolved_class_uri = f"{domain_root}#{self._sanitize_name(local)}" |
There was a problem hiding this comment.
Correctness + duplication. R2RMLGenerator.__init__ already normalizes base_uri to a trailing / (see test_init_normalizes_base_uri). Rebuilding as domain_root + '#' + sanitize(local) then disagrees with OWL (base_uri + class_name) and with instance templates (f"{self.base_uri}{class_name}/{{id}}").
startswith(domain_root) also keeps a hash URI that merely shares the path prefix, so hash-vs-slash drift is not actually fixed when the host/path is unchanged.
Please Extract Function this into URIHelpers (same helper as OWL + Digital Twin + PR 146). Use the generator's current self.base_uri separator, not a hard-coded #.
|
|
||
| # Ontology property-URI lookup for this class | ||
| ont_props = (data_prop_uri_lookup or {}).get(class_uri, {}) | ||
| ont_props = (data_prop_uri_lookup or {}).get(resolved_class_uri, {}) |
There was a problem hiding this comment.
Bug. _build_data_property_uri_lookup keys by the ontology class's stored uri. After rebase, resolved_class_uri no longer matches that key, so ont_props becomes {} and attribute predicates fall back to base_uri + sanitize(attr) — the opposite of what this change is trying to preserve.
Look up class_uri and resolved_class_uri (or rebase the lookup keys when building the map). Add a unit test: stale class URI + existing dataProperties[].uri still emits the ontology predicate.
| local = self._local_name(ref) | ||
| if local: | ||
| return URIRef(self.base_uri + local) | ||
| return URIRef(ref) |
There was a problem hiding this comment.
This rebase (base_uri + local) matches _add_class, which is good — keep that policy, just share it.
Two nits:
startswith(self.base_uri)is a prefix trap (http://ex.org/ontomatcheshttp://ex.org/ontology/...). Compare namespace + separator, or always rebuild from local name (what_add_classalready does).- The final
return URIRef(ref)still emits the stale URI when local name is empty. Drop it or log + returnNone.
The docstring is useful; once the helper exists this can shrink a lot.
| else: | ||
| # Stale class URI from a previous base_uri — rebuild using '#', | ||
| # matching how classes are declared in the OWL ontology. | ||
| full_class_uri = f"{domain_root}#{extract_local_name(class_uri)}" |
There was a problem hiding this comment.
Same copy-paste as R2RML (forced #, startswith(domain_root)). Call the shared helper.
Also: exclusion on line 313 still uses the unrebased class_uri, so a class excluded under the current Base URI can slip through if the mapping still stores the old URI.
| if 0 <= index < len(constraints): | ||
| constraints[index] = constraint | ||
| else: | ||
| constraints.append(constraint) |
There was a problem hiding this comment.
The frontend in ontology-shared-panels.js (saveEntityConstraintsToServer / saveRelationshipConstraintsToServer) has been posting here for a while — landing these endpoints is the right fix.
Please still:
- Move list mutate +
domain.save()ontoOntology(.cursor/07— routes stay thin). - Call
Ontology.validate_constraintbefore persist (already covers cardinality / characteristics / className). RaiseValidationErrorwith that message. - Treat only
index == -1as append. Any other out-of-range index should 400, same as delete. Today'selse: appendwill silently create duplicates if the UI ever sends a stale index.
Add API tests for save (append, replace, invalid payload) and delete (valid index, invalid index).
| if uri: | ||
| if uri and uri not in seen: | ||
| seen.add(uri) | ||
| result.append(uri) |
There was a problem hiding this comment.
Reading domain.constraints is the right source for the Designer tab. Matching by local name is fine.
Please drop the invent-URI fallback: a constraint whose property was renamed/removed should be logger.warning + skip. Building data_ns + prop_name will materialize triples for a predicate that does not exist in the ontology.
Nit: the tieneCapital example in the docstring is a domain name, not English commentary — fine if it stays an identifier, but a generic hasCapital keeps .cursor/05 §English-only artifacts unambiguous.
Add a unit test: {"type": "transitive", "property": "knows"} is found even when properties[].characteristics is empty.
What
Continuation of the "stale class URI after Base URI change" problem fixed for the reasoning engines in the companion reasoning-engines PR (
fix/reasoning-lakebase-dialect-and-stale-uri), applied here to ontology/mapping generation — plus an unrelated persistence gap in the Designer's Constraints tab that a couple of these fixes depend on.Stale-URI reconstruction, three places:
OntologyGenerator._resolve_uri: when given a full URI (not a simple name) — as happens when the Expressions & Axioms editor sends a class/property's stored URI — it now checks whether that URI starts with the domain's current Base URI, and if not, extracts the local name and rebuilds it against the current Base URI. Before this, an axiom referencing a stale class URI no longer matched that class's ownowl:Classdeclaration in the exported OWL, leaving the axiom effectively orphaned.R2RMLGenerator: reconstructs a mapping's class URI the same way before generating itsTriplesMap, and looks up the class's data properties using the resolved URI rather than the original. Before this, mapped instances got anrdf:typepointing at the stale URI — disconnected from the real class — and their data properties weren't found either.DigitalTwingraph construction: same reconstruction when building the visual graph, so instances of a re-based class attach to it correctly instead of appearing disconnected.Constraints tab had no backend to save to. Adds
POST /ontology/constraints/saveandPOST /ontology/constraints/delete, which persist/remove an entry (by index) indomain.constraints— entitydisjointWith/equivalentToand relationship cardinality/functional/inverse-functional/symmetric/transitive. Before this, the Constraints tab UI accepted input but had nowhere to send it, so nothing was ever saved.ReasoningServicedidn't consume Constraints tab data at all. Adds_get_constraints(), readingdomain.constraints(where the tab's data actually lives, via the new endpoints above) in addition to the property's internalcharacteristicsfield (which in practice the UI never populates). Before this, marking a relationship Transitive or Symmetric in the Constraints tab had no effect on graph reasoning — the transitive closure / symmetric expansion pass only looked at a field nothing wrote to.Why
Same root cause across the first three files: modules that trust a stored
uri/irifield literally instead of treating it as derivable from(local name, current Base URI). The Constraints-tab gap is separate but directly blocks the SWRL/graph-reasoning fix from being meaningfully testable, since without persistence there's nothing for_get_constraints()to read.How to test
rdf:typeshould point at the current-Base-URI class); open the Digital Twin view (instances should attach to the class).