Skip to content

fix(ontology): rebuild stale class URIs in OWL/R2RML/Digital Twin generation and persist Constraints tab - #147

Open
jeremiaspf wants to merge 1 commit into
databrickslabs:developfrom
jeremiaspf:fix/ontology-generation-stale-uri-and-constraints
Open

fix(ontology): rebuild stale class URIs in OWL/R2RML/Digital Twin generation and persist Constraints tab#147
jeremiaspf wants to merge 1 commit into
databrickslabs:developfrom
jeremiaspf:fix/ontology-generation-stale-uri-and-constraints

Conversation

@jeremiaspf

Copy link
Copy Markdown

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 own owl:Class declaration in the exported OWL, leaving the axiom effectively orphaned.
  • R2RMLGenerator: reconstructs a mapping's class URI the same way before generating its TriplesMap, and looks up the class's data properties using the resolved URI rather than the original. Before this, mapped instances got an rdf:type pointing at the stale URI — disconnected from the real class — and their data properties weren't found either.
  • DigitalTwin graph 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/save and POST /ontology/constraints/delete, which persist/remove an entry (by index) in domain.constraints — entity disjointWith/equivalentTo and 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.

ReasoningService didn't consume Constraints tab data at all. Adds _get_constraints(), reading domain.constraints (where the tab's data actually lives, via the new endpoints above) in addition to the property's internal characteristics field (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/iri field 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

  1. Rename a domain's Base URI after creating a class, then: save an axiom referencing that class from Expressions & Axioms and export the OWL (URIRef should match); run a mapping targeting that class and inspect the generated R2RML/loaded triples (rdf:type should point at the current-Base-URI class); open the Digital Twin view (instances should attach to the class).
  2. Open the Designer's Constraints tab, add an entity or relationship constraint, reload the page — it should still be there (previously lost).
  3. Mark a relationship Transitive (or Symmetric) in the Constraints tab, run graph reasoning, and check the transitive closure (or symmetric pairs) actually materialize.

…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.
@jeremiaspf
jeremiaspf requested a review from a team as a code owner August 22, 2026 09:27
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@benoitcayladbx benoitcayladbx self-assigned this Aug 23, 2026
@benoitcayladbx benoitcayladbx modified the milestones: v0/8/0, v0.8.0 Aug 23, 2026

@benoitcayladbx benoitcayladbx left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  1. Single rebase helper — Extract Function into URIHelpers (already has extract_local_name / is_uri). One policy for: current base_uri + local name, with the same separator the caller already uses (OntologyGenerator does base_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)). Prefix startswith(domain_root) is also too weak (http://ex.org/onto matches http://ex.org/ontology/...).
  2. R2RML data_prop_uri_lookup — lookup is still keyed by the stored class uri (_build_data_property_uri_lookup). Switching the get to resolved_class_uri misses after a rebase. Look up original and resolved, or rebase the lookup keys too.
  3. 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|delete round-trip + Ontology.validate_constraint rejection (tests/units/api/test_routes.py)
    • _find_properties_by_characteristic reads domain.constraints (transitive / symmetric)
  4. Changelogchangelogs/v0.8.0/<github-user>_YYYY-MM-DD.log (English). CI will block without it.
  5. CLA — still pending on this PR.

Should-fix

  • Routes: list mutate + domain.save() belongs on Ontology (.cursor/07 — thin routes). Call existing Ontology.validate_constraint (and raise ValidationError) before persist. Do not append when index is out of range — docstring says only -1 appends; 999 currently appends.
  • Missing-property fallback in _find_properties_by_characteristic should 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)}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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, {})

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This rebase (base_uri + local) matches _add_class, which is good — keep that policy, just share it.

Two nits:

  1. startswith(self.base_uri) is a prefix trap (http://ex.org/onto matches http://ex.org/ontology/...). Compare namespace + separator, or always rebuild from local name (what _add_class already does).
  2. The final return URIRef(ref) still emits the stale URI when local name is empty. Drop it or log + return None.

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)}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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() onto Ontology (.cursor/07 — routes stay thin).
  • Call Ontology.validate_constraint before persist (already covers cardinality / characteristics / className). Raise ValidationError with that message.
  • Treat only index == -1 as append. Any other out-of-range index should 400, same as delete. Today's else: append will 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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants