pyshifty 0.4: algebraic validation, template-guided repair, and shape-map extraction - #399
Open
gtfierro wants to merge 136 commits into
Open
pyshifty 0.4: algebraic validation, template-guided repair, and shape-map extraction#399gtfierro wants to merge 136 commits into
gtfierro wants to merge 136 commits into
Conversation
Wire Library.load(ontology_graph=...) through OntologyEnvironment so owl:imports are fetched by OntoEnv and stored in BuildingMOTIF's Oxigraph graph store rather than resolved manually. ShapeCollection resolve_imports and infer_templates now delegate to OntoEnv as well, removing the old recursive _resolve_imports helper.
pytest_generate_tests was calling setup_building_motif_brick/s223 at collection time, causing Brick.ttl, 223p.ttl, and all QUDT dependencies to be loaded into an in-memory SQLite DB just for --collect-only. Replace with cheap YAML-key + rdflib-local-parse extraction at collection time (~0.5s), and move the full Library.load into session-scoped fixtures that run once during test execution. Collection time drops from minutes to under 1s.
Add AlgebraicValidationContext that consumes pyshifty's algebraic output and drives soundness-gated, template-guided graph repair, wired through Model.validate(..., repair_libraries=...). Bump pyshifty to 0.2.0. Add two demo notebooks: pyshifty repair walkthrough and a pyshifty port of the existing-model validation example with hands-on repair sections.
…ntoenv - Add init_from_store=True to OntologyEnvironment so libraries already persisted in Oxigraph are visible to ontoenv import resolution on restart - Remove xsd:string-stripping branch from _decode_term; RDF 1.1 treats plain strings and xsd:string as equal, and the strip made triples() and query() return incompatible objects for the same stored value - Fix false-positive OntologyImportsNotFound in resolve_imports fallback path: track whether the fallback was used and check missing_imports against self.graph instead of graph_name to avoid index-based false positives - Add warning log to resolve_imports except-branch to match infer_templates - Wrap Oxigraph graph write in try/except in _load_from_ontology; on failure, clears any partial write and rolls back the SQL session for consistency - Forward infer_templates and run_shacl_inference through _load_from_directory to _load_shapes_from_directory (flags were silently ignored before) - Add try/finally to BuildingMOTIF.close() so all resources are released even if an earlier close step raises - Extract OntologyEnvironment.ensure_and_get_closure() helper to remove the duplicated ensure-registered+closure_copy pattern from resolve_imports and infer_templates - Update CLI docs to document --graph-store-path on load and serve subcommands - Update developer docs Python minimum version (3.10 -> 3.11)
Replace in-place graph mutation with copy-on-write. Model.replace_graph and ShapeCollection.replace_graph write new contents into a fresh graph_id and flip the SQL pointer, so a failure or session rollback leaves the previous graph intact instead of leaving a half-written graph. This removes the hand-rolled cleanup in Library.load and fixes the remove-then-add hazard in the model PUT endpoint. Add BuildingMOTIF.collect_graph_garbage to reclaim orphaned named graphs left by replacement and by row deletion. Only UUID-keyed graphs (models, shape collections, template bodies) are considered, so OntoEnv's IRI-keyed ontology graphs are excluded by construction. Runs best-effort on close(). Document the dual-store backend design (graph_id pointer, consistency model, copy-on-write, garbage collection) in a new explanations page.
Add GraphConnection.load_file_into_graph, which parses RDF files directly into a named graph using Oxigraph's native (Rust) loader instead of rdflib's per-triple Python path. On Brick.ttl (~54k triples) this is ~11x faster (0.80s -> 0.07s) and produces an identical stored graph. The library directory loader now uses it. Native loading is used only for trusted on-disk RDF (the native parser requires valid IRIs, unlike in-memory graphs that may carry generated template parameters), does not propagate file prefixes (callers re-bind the standard prefixes), and falls back to rdflib.parse if a file cannot be loaded natively, so behavior is never worse than before.
- test_database_persistence: snapshot store-backed graphs before bm.close() (Oxigraph deletes _store on close, making live views inaccessible); call BuildingMOTIF.clean() before reopening so the singleton is fresh; filter bare UUID graph identifiers out of BuildingMOTIFGraphStore.graph_ids() so OntoEnv's init_from_store does not choke on non-IRI names - dataclasses: add field(compare=False) to _bm and graph fields on Library, Template, Model, and ShapeCollection so __eq__ compares identity (id/name) rather than session pointers or graph object identity - notebooks/Session-example.ipynb: replace dead SQL table queries (kb_625d302a74_type_statements) with RDFLib graph API; graph data is now in Oxigraph, not the SQLite relational store - notebooks/223P-Validation.ipynb: fix relative path to 223p.ttl (notebooks/ runs as cwd, so ../libraries/... is correct)
- Replace _node_to_nt/_nt_to_node with node.n3()/from_n3() one-liners - _triples_to_graph uses direct g.add() instead of string concat + NT parse - Extract _make_resolve_library() to consolidate 4 duplicate Library.create() sites - Extract RepairWitness._get_summary()/_target() to remove copy-pasted callable-check blocks - witnesses_by_focus() uses defaultdict; get_broken_entities/get_diffs_for_entity delegate to cached diffset - get_shacl_backend() uses a registry dict; SHACL_ENGINES derived from it - compiled_model.validate() drops "default" sentinel; warns when repair_libraries ignored
In ontoenv 0.6.x, copy_closure and iter_closure_triples read from the native Rust store and bypass the custom graph_store adapter. When BuildingMOTIF's BuildingMOTIFGraphStore is in use, this returns empty graphs/iterators even though the ontology data is correctly stored. get_closure returns a ClosureGraphView that correctly routes through the adapter. Also bumps the BACnet integration test Dockerfile from python:3.10 to 3.11, matching the pyproject.toml minimum.
Template names only need to be unique within a library, so lookups and the ephemeral name-guarantee logic no longer need to treat a same-named template in a different library as a collision.
The repair engine ran an exponential TemplateMatcher monomorphism search against every template in repair_libraries, so a large library was slow regardless and the hard-coded MAX_TEMPLATES=25 cap truncated it arbitrarily by library order. Two of the four budgets (candidate_limit) were also unreachable because the context never threaded them to the engine. - Lift the four search budgets (max_templates, max_branches, build_fuel, candidate_limit) into a RepairConfig dataclass, threaded through Model.validate / CompiledModel.validate / AlgebraicValidationContext. Defaults reproduce the previous hard-coded values. max_templates=None disables the cap. candidate_limit is now reachable. - Filter repair_libraries for relevance before matching: for each failing hole, compute the rdf:types it requires and keep only templates whose name-type is comparable along rdfs:subClassOf (subclass -> mint, superclass -> reuse a more-specific node). Templates that type name only via a dependency are kept (conservative). The cap now applies to the filtered set, so it rarely binds; it warns once when it does. - Project the ontology to the triples TemplateMatcher actually reads (subClassOf + subPropertyOf + `a owl:Class`) and memoize per-template reuse candidates, both invariant across witnesses. Measured 3.4x faster on 81 trivial templates with identical proposals; the win scales with template size (2^nodes per template).
Add a `buildingmotif` skill under `.agents/skills/` covering the validate -> evidence -> repair -> re-validate loop, template and shape authoring, point label parsing, and ontology imports. Written against the installed `buildingmotif` package rather than a repo checkout, so it is usable by agents working in downstream projects.
The tests in tests/library manage the singleton by hand: the session-scoped setup fixtures build one instance with Brick (or 223P) already loaded, clear the singleton, and then each test re-installs that instance with `BuildingMOTIF.instance = bm`. Nothing put it back afterwards, so the last test to run in a process left a live singleton holding libraries named after the ontologies it had loaded. Constructing a BuildingMOTIF hands back the existing instance when there is one, so any later test whose fixture calls `BuildingMOTIF(...)` got that instance -- and its populated database -- rather than a fresh one. Running sequentially the unit tests all come first, so this was invisible; under `pytest -n auto` a worker can run a library test before a unit test, and tests/unit/dataclasses/test_library.py::test_libraries then fails with "UNIQUE constraint failed: library.name" while creating its Brick stand-in. Adds an autouse fixture in tests/library that drops the singleton after every test, and makes the `bm` fixture clean on setup as well as teardown, which is what clean_building_motif and the api building_motif fixture already did.
…search generate_all_subgraphs rebuilt the template's nx.DiGraph, and re-read T.all_nodes(), inside the inner loop -- once for each of the 2^|nodes| node subsets, though both are the same every time. `.subgraph()` returns a read-only view, so one conversion serves them all. On an 11-node template that is 2036 conversions instead of 1. TemplateMatcher._generate_mappings had the same shape and the more expensive version of it: every candidate subgraph built a _VF2SemanticMatcher, and each of those converted the whole *building* graph again. It now converts once and passes the result down through a new optional T_digraph argument; the matcher only reads the graph, so sharing one instance is safe. Output is unchanged -- with a fixed PYTHONHASHSEED, generate_all_subgraphs yields a byte-identical sequence before and after. Matching a 4-node guideline36 template against a 6k-triple model goes from 7.66s to 6.36s; the remainder is the VF2 search itself, which this does not touch.
The `libraries` field said it defaulted to the model's libraries. It defaults to empty: template guidance is opt-in through the `repair_libraries` argument of Model.validate / CompiledModel.validate. With no libraries the engine still proposes repairs, but only from recursive synthesis and pyshifty's own candidates -- the template reuse and mint sources contribute nothing, which is easy to miss when the comment says otherwise. Only the comment changes. Making the default match the old wording would turn template-guided repair on for every caller, which is a behavior change and wants its own decision.
The notebook does not pass a shacl_engine, so it now gets pyshifty, and validate() returns an AlgebraicValidationContext. as_templates() then runs the template-guided repair search over all 88 failing entities of the medium office model rather than the cheap GraphDiff path the cell was written against, and takes about 21 minutes -- well past nbmake's 600s per-cell timeout, so the integration test fails. Repairs the first couple of failures instead of all of them, the same way the cell further down tests only the first two Analytics_Application shapes. The cap counts repairs collected rather than entities visited: only 42 of the 88 failures yield a sound proposal and the first of those is the 47th, so stopping after N entities would produce nothing to show. Cheap failures are skipped in well under a millisecond each, so this stays fast. The notebook test goes from a 600s timeout to passing in 225s.
Take the algebraic and evidence surfaces pyshifty exposes rather than
re-deriving them on the BuildingMOTIF side.
`RepairWitness.target_shape` now reads the failing shape off the repair
witness itself. It used to read the *paired* violation, so it went None
whenever the (focus, statement_id, constraint_id) join came up empty --
pairing joins two independently computed results and is allowed to fail
(`alignment == "unavailable"`), but shape identity is carried on the
witness and does not depend on it.
`RepairWitness.missing_edges` states a cardinality failure in building
terms: which node is short, along which path, by how many, against which
qualifier -- enough to describe the edge that would close the deficit
without walking a repair tree or reading a SHACL report. Empty for a
failure that has a wrong value rather than a missing one.
`AlgebraicValidationContext.preview(proposal)` returns the validation run
the model would have if that proposal were applied, without mutating
anything and without rebuilding a context. Use it to *choose* between
proposals; `apply()`/`advance()` are for when you have chosen. Deletions
re-run SHACL-AF rules over the pre-inference graph, so a removed triple
takes its derivations with it instead of stranding them. The backing
EvidenceSession is built lazily, so a context that never previews a
repair never pays for one.
An empty shapes graph is now omitted rather than passed along: an
explicitly supplied zero-triple shapes graph is rejected outright
("explicit shapes graph is empty") instead of reporting vacuous
conformance. Omitting it takes the shapes from the data graph, which is
what an empty shape-collection list means here, and is what
`PyshiftyBackend.validate`/`.infer` already did. The W3C `report` path
takes the same shapes input rather than recomputing it.
The protocol for a native failure is spelled `ShiftyFailure` to keep it
distinct from `validation_result.Failure`, BuildingMOTIF's own
engine-independent protocol -- the one `RepairWitness` satisfies, as
opposed to the one it wraps.
A shape map is a binding table over a validation run: one entry per selected (shape, focus) pair, mapping each obligation the shape states to the values the model supplied for it. That is what `shape_to_df` and `shape_to_table` have always wanted -- a shape read as an extraction schema -- so build them on it. `CompiledModel.shape_map(shape)` exposes it directly. Values come from what SHACL matched, so `sh:or`, `sh:node`, sequence paths and qualified value shapes are honoured in full rather than approximated. Each entry additionally reports what a projection cannot: whether that focus conformed, how many qualifying values were wanted against how many were found, and which near-miss values were rejected. `shape_to_df` keeps its contract -- a `target` column plus one column per `sh:name`, one row per combination of bound values, conforming focus nodes only. `include_nonconforming=True` widens it to the focus nodes the shape selected but does not satisfy, with their unfilled slots left null; that is the more useful answer for "what is missing?" and the wrong one for "what is configured?", so it is opt-in. Column names are read from the shapes graph rather than from the map, for two reasons: a shape that selects no focus nodes still has to report its columns, and pyshifty 0.4.0 does not resolve `sh:name` when a node shape has exactly one property shape (it does with two or more). `ShapeCollection.shape_to_query` is unchanged and still the right call when you want a query -- to run elsewhere, to show someone, to embed. Its docstring now says that it is a translation, and therefore an approximation, and points at `shape_map` for the values themselves.
gtfierro
added a commit
that referenced
this pull request
Aug 30, 2026
Merge #37, plus three things that exist nowhere else: - `origin/gtf-new-pyshifty` had been overwritten with a `gtf-buildingmotif` commit, so PR #399 proposes all four features into `develop` rather than pyshifty alone. Not resolved here -- recorded, with what a clean rebuild would have to cherry-pick. - pyshifty 0.4.0 drops `sh:name` from a shape map when a node shape has exactly one property shape. `CompiledModel._slot_index` works around it; the note says to remove the fallback once it is fixed upstream. - Rows 32-36 of the merge table were never recorded.
gtfierro
changed the title
A SHACL-SPARQL body resolves its prefixed names against the prefix
declarations of the document shifty parses. `_shifty_shapes_input` has
always guaranteed that for a real shapes graph, but three call sites hand
shifty a data graph with *no* shapes argument -- and then the data graph
is also the shapes graph, so it needs exactly the same treatment.
It was not getting it, and the consequence was real:
`Library.from_ontology("Brick-full.ttl")` runs inference over a Brick
graph that our storage layer has already stripped the `ref:` binding
from, and Brick's own rules and constraints use
`ref:hasExternalReference`. Those queries were silently skipped -- the
rules never fired and nothing said so. pyshifty 0.4.1 raises
`Prefix not found` rather than skipping, which is what surfaced it.
`_shifty_data_input` re-binds prefixes for such a graph, guarded by
`_has_sparql_bodies` so the cost is only paid where it matters: the copy
and serialization run ~1.3s on Brick against ~3.3s for the inference
itself, and an ordinary model graph carries no SPARQL bodies and is
passed through untouched.
Applied to `PyshiftyBackend.infer`, `PyshiftyBackend.validate`, and the
empty-shape-collection branch of `AlgebraicValidationContext` (its
repair session, its algebra pass, and its W3C report path), which had
the identical latent bug.
The regression test fails without the fix under both pyshifty lines: on
0.4.0 the rule silently infers nothing, on 0.4.1 the call raises.
Note this restores BuildingMOTIF's *well-known* prefixes only. A custom,
downstream-defined namespace is still lost when its graph is persisted,
because the storage layer does not round-trip namespace bindings at all
-- now loudly rather than silently. That needs a storage change and is
out of scope here.
`_shifty_shapes_input` said a downstream-defined namespace was "gone the moment its shape collection is persisted", and that restoring it would mean round-tripping namespace bindings through the storage layer. That is wrong. SHACL's own prefix mechanism -- `sh:prefixes` pointing at a node carrying `sh:declare` -- is triples, not syntax, so it survives storage untouched, and shifty resolves it spec-compliantly. Verified end to end: a custom-prefixed `sh:construct` rule loaded through `Library.from_ontology`, read back with its `@prefix` bindings gone, still fires with no re-binding at all. `bind_prefixes` remains the fallback for graphs that only ever declared prefixes in Turtle syntax. Brick is one, and cannot be fixed from here: of its 80 SPARQL bodies, 79 point `sh:prefixes` at bare namespace IRIs carrying no `sh:declare`, its five real `sh:declare` triples hang off an ontology node no `sh:prefixes` references, and one rule has no `sh:prefixes` at all. Recorded on `_shifty_data_input`, which exists because of it.
A missing-value failure has two very different causes, and the report
could not tell them apart. A VAV whose air flow sensor is wired up but
mislabelled is one triple from conforming; a VAV with nothing on that
path needs a whole new node. `MissingEdge` now says which:
[air flow sensor] vav1 needs 1 more brick:hasPoint of class
brick:Air_Flow_Sensor (0 of 1 present); already on that path but not
qualifying: sen_a
Three new fields, sourced from a shape map built off the evidence
session's own run (`ShapeMap.from_run`) rather than a fresh validation,
and lazily, so a caller who never reads them never pays:
- `slot`: the shape author's `sh:name`, so a failure is reported in
their words rather than the engine's.
- `needs`: the required qualifier, kept as a *typed* pyshifty term
(`Cls`/`Datatype`/`ShapeRef`) so a caller can branch on the kind or
enumerate candidates for it, rather than flattened to a URIRef.
- `near_misses`: existing nodes the path already reaches that failed
`needs`.
Terms render as prefixed names -- `brick:hasPoint`, not the full IRI.
Graphs have usually lost their bindings by the time they reach a report,
so a shared namespace manager supplies BuildingMOTIF's well-known ones.
The join is semantic -- `(focus, path)`, disambiguated by the qualifier
class -- and deliberately NOT by `constraint_id`. Those ids are
session-local: the repair session and the evidence session number the
same constraint differently, and within a single run a shape-map binding
is numbered differently from the obligation beneath it. An id join does
not fail loudly, it attaches the wrong slot name and class to a real
finding. Ambiguity resolves to no enrichment, because reporting an
obligation plainly is always correct and mislabelling it is not.
Two exclusions that keep the output honest, both covered by tests:
- A value already serving a *satisfied* slot of the same focus is not a
near miss. The engine does report it as rejected for this slot, but
amending it would fix this obligation by breaking that one.
- Satisfied slots are not findings and are not reported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Integrates the
pyshiftySHACL engine at its 0.4 interface, and builds BuildingMOTIF'svalidation, repair and extraction surfaces directly on it. Pinned to
pyshifty>=0.4,<0.5.Validation and repair
pyshiftyas a SHACL engine alongsidepyshaclandtopquadrant, selected byshacl_engine=and normalized inbuildingmotif/shacl.py. New engine behaviour lives ina
ShaclBackendsubclass, not in the dataclasses.AlgebraicValidationContextreports failures asRepairWitnessobjects with native algebraic provenance (Reason.constraint,constraint_kind, and the(focus, statement_id, constraint_id)identity), rather thanre-deriving them from a flattened W3C report. It still satisfies the engine-independent
ValidationResult/Failureprotocols, so read loops work against either engine.RepairWitness.target_shapereads the failing shape off the witness itself(
Failure.shape_iri), so it is present for every witness — not only those whoseviolation could be paired.
RepairWitness.missing_edgesstates a cardinality failure as the edge that wouldclose it: which node is short, along which path, by how many, against which qualifier.
No repair tree to walk, no report to parse.
RepairConfig,RepairProposal, andcandidate generation from BuildingMOTIF templates via VF2 monomorphism, with every
candidate gated by pyshifty's soundness oracle.
AlgebraicValidationContext.preview(proposal)returns the run the model would haveunder
G ⊕ ΔG— pure, off a lazily builtEvidenceSession— so a caller can choosebetween proposals before applying one. Deletions re-run SHACL-AF rules over the
pre-inference graph, so a removed triple takes its derivations with it.
Extraction
CompiledModel.shape_map()reads a shape as an extraction schema: one entry perselected
(shape, focus)pair, mapping each obligation to the values the model supplied.Values come from what SHACL matched, so
sh:or,sh:node, sequence paths and qualifiedvalue shapes are honoured in full. Each entry also reports what a SPARQL projection
cannot — whether that focus conformed, expected against found counts, and rejected
near-miss values.
shape_to_df/shape_to_tableare built on it, keeping their contract (atargetcolumn plus one per
sh:name, conforming focus nodes only).include_nonconforming=Truewidens the frame to the focus nodes the shape selected but does not satisfy.
ShapeCollection.shape_to_queryis unchanged, and still the right call when you want aquery rather than the values.
Fixes carried here
sh:sparql/sh:rulebodies that silently neverfired because the shapes graph reached the engine without its prefix table.
supplied zero-triple shapes graph, so it is omitted instead.
Scope of this branch — please read before reviewing
The head of this PR is the
gtf-buildingmotifintegration branch, not a pyshifty-onlybranch. At some point
origin/gtf-new-pyshiftywas updated to a commit whose parent is agtf-buildingmotifmerge, so the diff againstdevelopis 126 commits across 117 filesand includes work that belongs to other PRs:
gtf-ontoenv(Replace graph store with Oxigraph + OntoEnv 0.6 #396) — ontology imports through OntoEnvgtf-uv(build: replace poetry with uv #398) — uv/PEP 621 packaginggtf-buildingmotif-skill— the agent skill under.agents/skills/gtf-manifestand the knowledge-base work — no PR openedReviewing this as a pyshifty change means reading past those. Splitting it back out means
rebuilding a branch from
developand cherry-picking the pyshifty commits;BRANCHES.mdon
gtf-buildingmotifrecords which ones and in what order.Known upstream issue
pyshifty0.4.0'sshape_map()does not resolvesh:namewhen a node shape has exactlyone property shape (it does with two or more).
CompiledModel._slot_indexreads slot namesfrom the shapes graph and matches them on
(path, qualifier)to work around it, preferringthe engine's own name whenever it is populated. Covered by
test_shape_to_df_names_a_single_slot_shape.Testing
Full unit suite green on the merge result: 607 passed, 3 skipped. Any test taking a
shacl_engineargument runs under all three engines.