perf: resolve a manifest's imports as one closure rooted at the manifest

A manifest is an ontology whose owl:imports name every member, so OntoEnv
can resolve the whole thing in one call instead of BuildingMOTIF
resolving each member's imports separately. Manifest.register() adds the
manifest graph to the ontology environment (idempotent, overwriting, ~4ms
for a graph of one triple per member) and Manifest.imports_closure()
returns the closure rooted at it: transitive and deduplicated, and flat
in the number of members where the old path was linear.

Measured on a Brick fixture + shape1.ttl (4,832-triple closure, 5 reps):

  members   one closure   per-collection   speedup
  1         0.080s        0.146s           1.8x
  2         0.077s        0.248s           3.2x
  4         0.082s        0.390s           4.8x

End to end through validate(), where SHACL dominates: 1.24x (1 member)
and 1.77x (2). Results are identical -- a test asserts the manifest path
and the per-collection path report the same failures for the same focus
nodes on every engine.

Each member is taken from exactly one source. The closure supplies the
members OntoEnv knows; a member it does not know -- a directory-loaded
library, whose name is not an ontology URI, or one built with
Library.create -- contributes its own shape collection instead. Never
both: OntoEnv's copy and the library's shape collection hold the same
triples with different blank-node labels, so unioning them would
duplicate every SHACL property shape and report each violation twice.
Anything that resolves neither way raises OntologyImportsNotFound, the
same exception resolve_imports has always raised, honoring
error_on_missing_imports.

CompiledModel now carries the Manifest its shape collections came from
and resolves the closure lazily in validate(), because compiling does not
need the imports at all. Passing an explicit list of shape collections
keeps the per-collection path -- a bare list has no manifest to root a
closure at.

Inference input is deliberately unchanged: compile() still runs inference
against the member shape collections, not the closure. What a model
infers from should be the shapes it was compiled against, and pulling
every transitively imported ontology into the inference input would
change what lands in every compiled model.

Full unit suite: 570 passed, 1 skipped.

Author: gtfierro

.agents/skills/buildingmotif/references/validation.md

@@ -394,6 +394,11 @@ Two things in this script that trip people up:
   validate against the manifest plus extras, spread `model.manifest.shape_collections()`
   into the list. `model.validate()` with no list validates against the manifest alone —
   that is, against the shape collections of every library the manifest names.
+- **The no-argument form is also the faster one.** A manifest is an ontology whose
+  `owl:imports` name its members, so `model.validate()` resolves them as a single OntoEnv
+  closure rooted at the manifest instead of resolving each collection's imports separately
+  — flat in the number of members rather than linear. Passing an explicit list gives up
+  that path, since a bare list has no manifest to root a closure at.
 - **`error_on_missing_imports=False` is the notebooks' default for real models.** A real
   model's shapes usually `owl:imports` something you haven't loaded; with `True` (the
   default) validation raises and stops. `False` gets you a report now — but always check

buildingmotif/dataclasses/compiled_model.py

@@ -22,6 +22,7 @@
 if TYPE_CHECKING:
     from buildingmotif.dataclasses.algebraic_validation import RepairConfig
     from buildingmotif.dataclasses.library import Library
+    from buildingmotif.dataclasses.manifest import Manifest
 
 
 @dataclass
@@ -40,6 +41,7 @@ def __init__(
         shape_collections: List[ShapeCollection],
         compiled_graph: rdflib.Graph,
         shacl_engine: Optional[str] = None,
+        manifest: Optional["Manifest"] = None,
     ):
         """
         :param shacl_engine: the engine this model was compiled with. None
@@ -48,9 +50,16 @@ def __init__(
             sentinel the rest of the codebase uses -- ``Model.compile`` and
             ``Model.validate`` both already take ``Optional[str]``.
         :type shacl_engine: Optional[str]
+        :param manifest: the manifest ``shape_collections`` came from, when it
+            came from one. :py:meth:`validate` then resolves imports as a
+            single OntoEnv closure rooted at the manifest rather than once per
+            collection. It is held rather than resolved here because compiling
+            does not need the imports at all -- only validating does.
+        :type manifest: Optional[Manifest]
         """
         self.model = model
         self.shape_collections = shape_collections
+        self.manifest = manifest
         self.shacl_engine = (
             self.model._bm.shacl_engine
             # "default" is the legacy spelling of "inherit from the singleton"
@@ -205,6 +214,16 @@ def validate(
             )
         backend = get_shacl_backend(shacl_engine)
 
+        # One closure rooted at the manifest, rather than one resolve_imports
+        # per collection: the manifest names every collection as an
+        # owl:imports, so OntoEnv can do the whole transitive resolution in a
+        # single call and deduplicate it on the way.
+        resolved_shapes = (
+            self.manifest.imports_closure(error_on_missing=error_on_missing_imports)
+            if self.manifest is not None
+            else None
+        )
+
         # The pyshifty engine exposes a native algebraic + symbolic-repair API.
         # Auto-route it to the AlgebraicValidationContext, which computes repairs
         # by abduction over the algebra and gates every one for soundness, rather
@@ -219,6 +238,7 @@ def validate(
                 self._compiled_graph,
                 self.shape_collections,
                 error_on_missing_imports=error_on_missing_imports,
+                resolved_shapes=resolved_shapes,
             )
             return AlgebraicValidationContext.from_compiled(
                 self.shape_collections,
@@ -235,6 +255,7 @@ def validate(
             self._compiled_graph,
             self.shape_collections,
             error_on_missing_imports=error_on_missing_imports,
+            resolved_shapes=resolved_shapes,
         )
         return ValidationContext(
             self.shape_collections,

buildingmotif/dataclasses/manifest.py

@@ -26,6 +26,7 @@
 from buildingmotif.database.errors import LibraryNotFound
 from buildingmotif.dataclasses.library import Library
 from buildingmotif.dataclasses.shape_collection import ShapeCollection
+from buildingmotif.ontology_environment import OntologyImportsNotFound
 
 if TYPE_CHECKING:
     from buildingmotif.dataclasses.model import Model
@@ -354,6 +355,90 @@ def shape_collections(self, error_on_missing: bool = True) -> List[ShapeCollecti
             for library in self.resolve(error_on_missing=error_on_missing)
         ]
 
+    def register(self) -> str:
+        """Register this manifest's graph with the ontology environment.
+
+        The manifest *is* an ontology -- a declaration and a list of
+        ``owl:imports`` -- so OntoEnv can resolve it like any other, which is
+        what makes :py:meth:`imports_closure` a single call. Registering is
+        idempotent and overwrites, so it is cheap to redo rather than track
+        whether the manifest changed since last time (the graph is one triple
+        per member).
+
+        :return: the name OntoEnv registered it under, i.e. :py:attr:`uri`
+        :rtype: str
+        """
+        # OntoEnv names a graph by its owl:Ontology declaration, so a manifest
+        # that has never been added to has nothing to register *as*.
+        self._ensure_declared()
+        return self._model._bm.ontology_environment.add(
+            self.graph, fetch_imports=False, overwrite=True
+        )
+
+    def imports_closure(self, error_on_missing: bool = True) -> Graph:
+        """The merged shapes graph this model is validated against.
+
+        One OntoEnv closure rooted at this manifest, rather than resolving each
+        member's imports separately: the manifest declares every member as an
+        ``owl:imports``, so the closure is exactly "every graph this model is
+        checked against", already transitive and already deduplicated.
+
+        Members OntoEnv cannot resolve -- a directory-loaded library, whose
+        name is not an ontology URI, or one built with :py:meth:`Library.create`
+        -- come from their own shape collections instead. Each member is taken
+        from **one** source, never both: OntoEnv's copy of an ontology and the
+        library's shape collection hold the same triples but relabel blank
+        nodes, so unioning them would duplicate every SHACL property shape and
+        report each violation twice.
+
+        :param error_on_missing: if True (default), an import that resolves
+            neither through OntoEnv nor as a library raises, as
+            :py:meth:`ShapeCollection.resolve_imports` has always done. If
+            False they are logged and skipped.
+        :type error_on_missing: bool
+        :raises OntologyImportsNotFound: per ``error_on_missing``
+        :return: the shapes graph
+        :rtype: Graph
+        """
+        if not self.imports:
+            # An empty manifest asks nothing of the model. Short-circuit rather
+            # than round-tripping through OntoEnv, which would have no graph to
+            # root a closure at anyway.
+            return Graph()
+
+        env = self._model._bm.ontology_environment
+        self.register()
+
+        closure, _ = env.closure_copy(str(self.uri))
+
+        # Whatever the closure could not reach: manifest members OntoEnv does
+        # not know, plus anything transitively imported and missing (which
+        # resolve_imports has always treated as an error).
+        unresolved = []
+        for iri in env.missing_imports(str(self.uri)):
+            name = library_name(URIRef(iri))
+            try:
+                library = self._resolve_one(name)
+            except ManifestLibraryNotFound:
+                unresolved.append(iri)
+                continue
+            closure += (
+                library.get_shape_collection()
+                .resolve_imports(error_on_missing_imports=error_on_missing)
+                .graph
+            )
+
+        if unresolved:
+            if error_on_missing:
+                raise OntologyImportsNotFound(unresolved)
+            logger.warning(
+                "Manifest of %s could not resolve: %s. Validation will not see "
+                "any shapes those graphs define.",
+                self._model.name,
+                ", ".join(sorted(unresolved)),
+            )
+        return closure
+
     def _ensure_declared(self) -> None:
         """Give the manifest graph an ``owl:Ontology`` declaration if it has none."""
         graph = self._shape_collection.graph

buildingmotif/dataclasses/model.py

@@ -285,11 +285,15 @@ def validate(
             so code that only reads failures need not care which one it got.
         :rtype: ValidationResult
         """
+        manifest = None
         if not shape_collections:
-            shape_collections = self.manifest.shape_collections(
+            manifest = self.manifest
+            shape_collections = manifest.shape_collections(
                 error_on_missing=error_on_missing_imports
             )
-        compiled_model = self.compile(shape_collections, shacl_engine=shacl_engine)
+        compiled_model = self.compile(
+            shape_collections, shacl_engine=shacl_engine, manifest=manifest
+        )
         return compiled_model.validate(
             error_on_missing_imports,
             shacl_engine,
@@ -301,6 +305,7 @@ def compile(
         self,
         shape_collections: Optional[List["ShapeCollection"]] = None,
         shacl_engine: Optional[str] = None,
+        manifest: Optional["Manifest"] = None,
     ) -> "CompiledModel":
         """Compile the graph of a model against a set of ShapeCollections.
 
@@ -310,21 +315,33 @@ def compile(
         :param shacl_engine: the SHACL engine to use for validation, defaults to whatever
             is set in the BuildingMOTIF object
         :type shacl_engine: str, optional
+        :param manifest: the manifest ``shape_collections`` came from, if any.
+            Passed on to the :py:class:`CompiledModel` so that validating it
+            later resolves imports as one closure rooted at the manifest. It is
+            filled in automatically when ``shape_collections`` is omitted.
+        :type manifest: Optional[Manifest]
         :return: copy of model's graph that has been compiled against the
             ShapeCollections
         :rtype: Graph
         """
         from buildingmotif.dataclasses.compiled_model import CompiledModel
 
         if shape_collections is None:
-            shape_collections = self.manifest.shape_collections()
+            manifest = self.manifest
+            shape_collections = manifest.shape_collections()
         backend = get_shacl_backend(shacl_engine or self._bm.shacl_engine)
+        # NB: inference compiles against the member shape collections
+        # themselves, *not* the manifest's imports closure -- what a model
+        # infers from should be the shapes it was compiled against, and pulling
+        # every transitively imported ontology into the inference input would
+        # change what lands in every compiled model.
         compiled_graph = backend.compile_model_graph(self.graph, shape_collections)
         return CompiledModel(
             self,
             shape_collections,
             compiled_graph,
             shacl_engine=shacl_engine,
+            manifest=manifest,
         )
 
     @property

buildingmotif/shacl.py

@@ -106,6 +106,30 @@ def _shifty_shapes_input(shape_graph: Graph) -> bytes:
     return prefixed.serialize(format="turtle", encoding="utf-8")
 
 
+def _resolved_shape_graph(
+    shape_collections: List["ShapeCollection"],
+    error_on_missing_imports: bool,
+    resolved_shapes: Optional[Graph],
+) -> Graph:
+    """The shapes graph, with ``owl:imports`` resolved.
+
+    ``resolved_shapes`` is the whole answer when it is given: validating
+    against a manifest computes one OntoEnv closure rooted at the manifest
+    (:py:meth:`Manifest.imports_closure`), which is transitive, deduplicated,
+    and one call rather than one per collection. The per-collection path
+    remains for an explicit list of shape collections, which has no manifest to
+    root a closure at.
+    """
+    if resolved_shapes is not None:
+        return resolved_shapes
+    graph = Graph()
+    for shape_collection in shape_collections:
+        graph += shape_collection.resolve_imports(
+            error_on_missing_imports=error_on_missing_imports
+        ).graph
+    return graph
+
+
 @dataclass
 class ValidationGraphs:
     data_graph: Graph
@@ -146,18 +170,24 @@ def validation_graphs(
         compiled_graph: Graph,
         shape_collections: List["ShapeCollection"],
         error_on_missing_imports: bool = True,
+        resolved_shapes: Optional[Graph] = None,
     ) -> ValidationGraphs:
+        """
+        :param resolved_shapes: the shapes graph with its ``owl:imports``
+            already resolved -- see :func:`_resolved_shape_graph`. Defaults to
+            resolving each shape collection's imports separately.
+        :type resolved_shapes: Optional[Graph]
+        """
         from buildingmotif.utils import (
             copy_graph,
             rewrite_shape_graph,
             skolemize_shapes,
         )
 
         graph = copy_graph(compiled_graph)
-        for shape_collection in shape_collections:
-            graph += shape_collection.resolve_imports(
-                error_on_missing_imports=error_on_missing_imports
-            ).graph
+        graph += _resolved_shape_graph(
+            shape_collections, error_on_missing_imports, resolved_shapes
+        )
 
         graph = rewrite_shape_graph(graph)
         graph.remove((None, OWL.imports, None))
@@ -170,9 +200,13 @@ def validate_compiled_model(
         compiled_graph: Graph,
         shape_collections: List["ShapeCollection"],
         error_on_missing_imports: bool = True,
+        resolved_shapes: Optional[Graph] = None,
     ) -> Tuple[ValidationResult, Graph]:
         graphs = self.validation_graphs(
-            compiled_graph, shape_collections, error_on_missing_imports
+            compiled_graph,
+            shape_collections,
+            error_on_missing_imports,
+            resolved_shapes=resolved_shapes,
         )
         return (
             self.validate(graphs.data_graph, graphs.shape_graph),
@@ -303,17 +337,16 @@ def validation_graphs(
         compiled_graph: Graph,
         shape_collections: List["ShapeCollection"],
         error_on_missing_imports: bool = True,
+        resolved_shapes: Optional[Graph] = None,
     ) -> ValidationGraphs:
         # As in compile_model_graph, the shapes and data are handed to shifty
         # un-skolemized and un-rewritten (no sh:node inlining): shifty consumes
         # the native SHACL algebra directly rather than a flattened shape graph.
         from buildingmotif.utils import copy_graph
 
-        shape_graph = Graph()
-        for shape_collection in shape_collections:
-            shape_graph += shape_collection.resolve_imports(
-                error_on_missing_imports=error_on_missing_imports
-            ).graph
+        shape_graph = _resolved_shape_graph(
+            shape_collections, error_on_missing_imports, resolved_shapes
+        )
 
         data_graph = copy_graph(compiled_graph)
         return ValidationGraphs(data_graph, shape_graph, shape_graph)

docs/explanations/manifests.md

@@ -85,8 +85,43 @@ what it cannot resolve instead of raising.
 
 `model.manifest.shape_collections()` is the list `validate()` and `compile()` use: the
 shape collection of each member library, in name order. The manifest's own graph is *not*
-in that list — it holds imports, not shapes. Each member's own `owl:imports` are resolved
-as they always were, through OntoEnv, at validation time.
+in that list — it holds imports, not shapes.
+
+Resolving those members' own `owl:imports` is **one OntoEnv closure rooted at the
+manifest**, not one resolution per member. This falls out of the storage format: a manifest
+is an ontology whose `owl:imports` name every member, so registering it with OntoEnv
+(`manifest.register()`, done automatically) makes "every graph this model is checked
+against" a single `closure` call — already transitive, already deduplicated.
+`manifest.imports_closure()` is that graph.
+
+Measured on a Brick fixture + `shape1.ttl` (4,832-triple closure, 5 reps, warm):
+
+| manifest | one closure | per-collection resolve | speedup |
+|---|---|---|---|
+| 1 member | 0.080s | 0.146s | 1.8x |
+| 2 members | 0.077s | 0.248s | 3.2x |
+| 4 members | 0.082s | 0.390s | 4.8x |
+
+The closure's cost is flat in the number of members; the per-collection path is linear,
+because each member re-resolves imports the others may share. Registering the manifest
+costs ~3.7ms and happens on each call, so a manifest edited between validations is never
+validated against its old membership.
+
+Two things are deliberate here:
+
+- **Each member is taken from exactly one source.** For a member OntoEnv knows, the closure
+  supplies it; for one it does not — a directory-loaded library, or anything built with
+  `Library.create` — the library's own shape collection is unioned in instead. Never both:
+  OntoEnv's copy and the library's shape collection hold the same triples but relabel blank
+  nodes, so unioning them would duplicate every SHACL property shape and report each
+  violation twice.
+- **Inference input is unchanged.** `compile()` still runs inference against the member
+  shape collections themselves, not the closure. What a model infers from should be the
+  shapes it was compiled against; pulling every transitively imported ontology into the
+  inference input would change what lands in every compiled model.
+
+An explicit list — `model.validate([sc1, sc2])` — has no manifest to root a closure at, so
+it resolves each collection's imports as before.
 
 Passing shape collections explicitly still bypasses the manifest entirely:
 `model.validate([sc1, sc2])` checks against exactly those. To validate against the