From 9ca83d397f9067a6ccd84335e1e8d7b07902dae7 Mon Sep 17 00:00:00 2001 From: FABIOTESS Date: Wed, 19 Aug 2026 17:35:06 +0100 Subject: [PATCH] fix(export): address review findings on the ontology schema fix Four findings from the automated review, all reproduced first. 1. The name fallback minted invalid IRIs. `_term_iri` pasted a raw name onto the ontology base, so a class named "Customer Account" produced . rdflib only warns about the space, Oxigraph rejects it with "Invalid IRI code point". That is the same class of defect this PR set out to fix, introduced by the fix itself. Local names are now percent-encoded. 2. `improve_coherence` raised AttributeError. It lives on OntologyOptimizer, which holds no namespace manager, so the URI fallback I added there crashed on any ontology carrying a class without a URI. It now mints from the ontology's own base through a shared module-level helper. 3. `owl:Thing` was treated as an absolute IRI. It matches the generic scheme grammar, so `_is_absolute_iri` accepted it and domains and ranges came out as the term rather than . This is the live path: stage 4 of the generator assigns ["owl:Thing"] to object properties with no inferred endpoints. Absoluteness is now decided on a real scheme, and the well-known prefixes expand. 4. Unusable property entries were dropped in silence. Non-dictionary entries and definitions carrying no type are now named in a warning. 6 further tests, including a strict-parser check through Oxigraph, which is what catches the space that rdflib waves through. --- semantica/export/owl_exporter.py | 74 +++++++++++- semantica/ontology/ontology_generator.py | 29 ++++- .../test_owl_exporter_generator_schema.py | 110 ++++++++++++++++++ 3 files changed, 206 insertions(+), 7 deletions(-) diff --git a/semantica/export/owl_exporter.py b/semantica/export/owl_exporter.py index 6770ddeb..64614de2 100644 --- a/semantica/export/owl_exporter.py +++ b/semantica/export/owl_exporter.py @@ -25,6 +25,7 @@ License: MIT import re from datetime import datetime from pathlib import Path +from urllib.parse import quote from typing import Any, Dict, List, Optional, Union from ..utils.exceptions import ProcessingError, ValidationError @@ -38,6 +39,9 @@ from ..utils.progress_tracker import get_progress_tracker # PROV-exported URIs co-resolve under one shared namespace by default. from ..provenance.manager import DEFAULT_BASE_URI +#: Module-level logger, for the classmethod helpers that have no instance. +logger = get_logger("owl_exporter") + class OWLExporter: """ @@ -374,15 +378,61 @@ class OWLExporter: _XSD_NS = "http://www.w3.org/2001/XMLSchema#" + #: Prefixes the generator and hand-authored ontologies actually use. A + #: prefixed name is not an absolute IRI: `owl:Thing` matches the generic + #: scheme grammar, so treating it as one produced as a domain, + #: which is a different term from http://www.w3.org/2002/07/owl#Thing. + _KNOWN_PREFIXES = { + "owl": "http://www.w3.org/2002/07/owl#", + "rdf": "http://www.w3.org/1999/02/22-rdf-syntax-ns#", + "rdfs": "http://www.w3.org/2000/01/rdf-schema#", + "xsd": _XSD_NS, + "skos": "http://www.w3.org/2004/02/skos/core#", + "dc": "http://purl.org/dc/elements/1.1/", + "dcterms": "http://purl.org/dc/terms/", + "foaf": "http://xmlns.com/foaf/0.1/", + "sem": "https://semantica.dev/ns#", + "semantica": "https://semantica.dev/ns#", + } + + #: Schemes that really do introduce an absolute IRI without `//`. + _ABSOLUTE_SCHEMES = ("urn:", "doi:", "mailto:", "tag:", "uuid:") + + @classmethod + def _is_absolute_iri(cls, value: str) -> bool: + if not isinstance(value, str): + return False + value = value.strip() + if "://" in value: + return bool(re.match(r"^[A-Za-z][A-Za-z0-9+.\-]*://", value)) + return value.lower().startswith(cls._ABSOLUTE_SCHEMES) + + @classmethod + def _expand_prefixed_name(cls, value: str) -> str: + """Expand a known prefixed name, or return "" when it cannot be expanded.""" + prefix, _, local = value.partition(":") + namespace = cls._KNOWN_PREFIXES.get(prefix) + return f"{namespace}{local}" if namespace and local else "" + @staticmethod - def _is_absolute_iri(value: str) -> bool: - return bool(re.match(r"^[A-Za-z][A-Za-z0-9+.\-]*:", value)) + def _iri_safe(local: str) -> str: + """ + Percent-encode a local name so it can sit inside <>. + + A name is free text. "Customer Account" pasted onto a base gives an IRI + with a space in it, which rdflib only warns about and Oxigraph rejects + with "Invalid IRI code point". + """ + return quote(local.strip(), safe="~._-!$&'()*+,;=:@/?") @classmethod def _join_iri(cls, base: str, local: str) -> str: """Append a local name to a base IRI, respecting hash and slash bases.""" if not base: return "" + local = cls._iri_safe(local) + if not local: + return "" separator = "" if base.endswith(("#", "/", ":")) else "#" return f"{base}{separator}{local}" @@ -438,8 +488,8 @@ class OWLExporter: return value if value in index: return index[value] - if ":" in value: # a prefixed name we cannot expand - return "" + if ":" in value: + return cls._expand_prefixed_name(value) return cls._join_iri(base, value) @classmethod @@ -497,16 +547,32 @@ class OWLExporter: if isinstance(prop, dict): data_props.append(prop) + skipped = 0 + untyped = [] for prop in ontology.get("properties", []) or []: if not isinstance(prop, dict): + skipped += 1 continue kind = str(prop.get("type") or "").strip().lower() owl_type = str(prop.get("@type") or "").strip().lower() if kind in ("object", "objectproperty") or owl_type.endswith("objectproperty"): object_props.append(prop) else: + if not kind and not owl_type: + untyped.append(prop.get("name") or prop.get("uri") or "") data_props.append(prop) + if skipped: + logger.warning( + f"Skipped {skipped} entr(y/ies) in 'properties' that are not " + "dictionaries and cannot be exported" + ) + if untyped: + logger.warning( + "Exported as data properties because they declare no type or " + f"@type: {', '.join(str(name) for name in untyped)}" + ) + return object_props, data_props @staticmethod diff --git a/semantica/ontology/ontology_generator.py b/semantica/ontology/ontology_generator.py index db5ae460..819f1b94 100644 --- a/semantica/ontology/ontology_generator.py +++ b/semantica/ontology/ontology_generator.py @@ -33,6 +33,7 @@ License: MIT from dataclasses import dataclass, field, replace as dataclass_replace from datetime import datetime from typing import Any, Dict, List, Optional +from urllib.parse import quote from ..utils.exceptions import ProcessingError, ValidationError from ..utils.logging import get_logger @@ -698,12 +699,15 @@ class OntologyOptimizer: # Ensure all classes have required fields. Both guards used `not in`, # which misses a key that is present and None, and the URI fallback # assigned a bare class name where an absolute IRI is required (#1103). + # + # The base comes from the ontology being optimized. OntologyOptimizer + # holds no namespace manager, so reaching for one here would raise + # AttributeError on every ontology carrying a class with no URI. + base_uri = ontology.get("uri") or DEFAULT_ONTOLOGY_BASE_URI classes = ontology.get("classes", []) for cls in classes: if not cls.get("uri"): - cls["uri"] = self.namespace_manager.generate_class_iri( - cls.get("name", "Entity") - ) + cls["uri"] = _mint_term_iri(base_uri, cls.get("name", "Entity")) if not cls.get("label"): cls["label"] = cls.get("name", "Entity") @@ -751,6 +755,25 @@ class NodeShape: severity: str = "Violation" +#: Used when an ontology carries no URI of its own. +DEFAULT_ONTOLOGY_BASE_URI = "https://semantica.dev/ontology/" + + +def _mint_term_iri(base_uri: str, name: str) -> str: + """ + Mint an absolute IRI for a term from a base and a name. + + The name is percent-encoded: names are free text, and "Customer Account" + pasted onto a base gives an IRI with a space in it, which strict parsers + reject outright. + """ + local = quote(str(name).strip(), safe="~._-!$&'()*+,;=:@") + if not local: + local = "Entity" + separator = "" if base_uri.endswith(("#", "/", ":")) else "#" + return f"{base_uri}{separator}{local}" + + @dataclass class SHACLGraph: """Internal model representing the complete SHACL shapes graph.""" diff --git a/tests/export/test_owl_exporter_generator_schema.py b/tests/export/test_owl_exporter_generator_schema.py index db65ca70..d925fe6b 100644 --- a/tests/export/test_owl_exporter_generator_schema.py +++ b/tests/export/test_owl_exporter_generator_schema.py @@ -179,3 +179,113 @@ def test_a_class_without_any_identifier_is_skipped_not_emitted_as_empty(): assert set(graph.subjects(RDF.type, OWL.Class)) == set() assert (URIRef("https://example.org/onto/"), RDF.type, OWL.Ontology) in graph + + +# ── Review findings on the first revision of this fix ──────────────────────── + +def test_a_name_with_a_space_still_mints_a_valid_iri(): + """The name fallback pasted free text onto a base, producing `<... ...>`.""" + import pyoxigraph + + ontology = { + "uri": "https://example.org/onto/", + "name": "Spaces", + "classes": [{"name": "Customer Account"}], + } + turtle = OWLExporter()._export_owl_turtle(ontology) + + graph = Graph() + graph.parse(data=turtle, format="turtle") + subjects = [str(s) for s in graph.subjects(RDF.type, OWL.Class)] + assert subjects, "the class was dropped entirely" + assert " " not in subjects[0], subjects[0] + + # rdflib only warns about a space in an IRI; a strict parser refuses it. + pyoxigraph.Store().load( + turtle.encode(), format=pyoxigraph.RdfFormat.TURTLE, base_iri=None + ) + + +def test_owl_thing_expands_instead_of_becoming_its_own_scheme(): + """`owl:Thing` matches the generic scheme grammar but is a prefixed name.""" + ontology = { + "uri": "https://example.org/onto/", + "name": "Thing", + "classes": [{"name": "Person", "uri": "https://example.org/onto/Person"}], + "object_properties": [ + { + "name": "relatedTo", + "uri": "https://example.org/onto/relatedTo", + "domain": ["owl:Thing"], + "range": ["owl:Thing"], + } + ], + } + graph = Graph() + graph.parse(data=OWLExporter()._export_owl_turtle(ontology), format="turtle") + + subject = URIRef("https://example.org/onto/relatedTo") + for predicate in (RDFS.domain, RDFS.range): + values = [str(v) for v in graph.objects(subject, predicate)] + assert values == ["http://www.w3.org/2002/07/owl#Thing"], values + + +def test_the_generators_owl_thing_fallback_round_trips(): + """stage 4 assigns domain/range of ["owl:Thing"], so this is the live path.""" + ontology = { + "uri": "https://example.org/onto/", + "name": "Generated", + "classes": [{"name": "Person", "uri": "https://example.org/onto/Person"}], + "properties": [ + {"name": "linkedTo", "type": "object", "uri": "https://example.org/onto/linkedTo", + "domain": ["owl:Thing"], "range": ["owl:Thing"], "@type": "owl:ObjectProperty"} + ], + } + graph = Graph() + graph.parse(data=OWLExporter()._export_owl_turtle(ontology), format="turtle") + + assert ( + URIRef("https://example.org/onto/linkedTo"), + RDFS.domain, + URIRef("http://www.w3.org/2002/07/owl#Thing"), + ) in graph + + +def test_optimizing_a_class_without_a_uri_does_not_raise(): + """improve_coherence lives on OntologyOptimizer, which owns no namespace manager.""" + from semantica.ontology.ontology_generator import OntologyOptimizer + + result = OntologyOptimizer().improve_coherence( + {"uri": "https://example.org/onto/", "classes": [{"name": "Person"}], "properties": []} + ) + minted = result["classes"][0]["uri"] + assert minted.startswith("https://example.org/onto/"), minted + assert " " not in minted + + +def test_optimizing_falls_back_to_a_base_when_the_ontology_has_no_uri(): + from semantica.ontology.ontology_generator import OntologyOptimizer + + result = OntologyOptimizer().improve_coherence( + {"classes": [{"name": "Customer Account"}], "properties": []} + ) + minted = result["classes"][0]["uri"] + assert minted.startswith("http"), minted + assert " " not in minted, minted + + +def test_unusable_property_entries_are_reported_not_silently_dropped(caplog): + import logging + + ontology = { + "uri": "https://example.org/onto/", + "name": "Malformed", + "classes": [], + "properties": ["not a dict", {"name": "untyped", "uri": "https://example.org/onto/untyped"}], + } + with caplog.at_level(logging.WARNING): + OWLExporter()._export_owl_turtle(ontology) + + messages = " ".join(record.getMessage() for record in caplog.records) + assert "not dictionaries" in messages or "not\ndictionaries" in messages or "dictionaries" in messages + assert "untyped" in messages