Skip to content

fix(ontology): validate SHACL term kinds - #622

Closed
seonghobae wants to merge 10 commits into
mainfrom
feat/ontology-repository-case-namespace
Closed

fix(ontology): validate SHACL term kinds#622
seonghobae wants to merge 10 commits into
mainfrom
feat/ontology-repository-case-namespace

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • validate SHACL target classes only against declared or entailed ontology classes
  • validate SHACL paths only against declared properties or standard reification paths
  • align the corporate-entity UI fixture with the canonical ontology annotation

Validation

  • uv run --extra dev pytest tests/test_publish_ontology_site.py -q (14 passed)
  • uv run --extra dev pytest tests/test_public_docstrings.py -q (2 passed)
  • targeted App regression passed with 92 unrelated tests skipped
  • frontend oxlint passed

Follow-up to #616 for review findings completed after its protected auto-merge.


Open in Devin Review

…R 0205)

Resolves #372. Supersedes ADR 0157: the repository-case public
namespace https://contextualwisdomlab.github.io/LineageWeave/ontology#
is the exact project path GitHub Pages serves and becomes the only
spelling new runtime values, exports, fixtures, and database rows mint.
The lowercase form stays published as a deprecated compatibility
vocabulary with validated equivalentClass/equivalentProperty mappings,
and scripts/migrate_legacy_namespace.py rewrites stored lowercase IRIs
(dry-run by default; never touches provenance columns).

Also lands the completeness increments that motivated the decision:
grounded node-attribute datatype properties, the SKOS post-type scheme,
person-side disjointness plus inverse affiliation, and a closed-world
SHACL shapes graph (docs/ontology/lineageweave-kg-shapes.ttl) validated
against every DB-to-RDF projection and published beside the ontology.

Full unit suite: 1095 passed, 12 skipped.
…ory-case-namespace

# Conflicts:
#	CHANGELOG.md
#	pyproject.toml
…as role=status

Its resolved empty/unavailable state carried role="status" like sibling
panels' transient loading text does, so mounting it inside the Board's
collapsed Advanced Review Tools details collided with every other status
region on the page (4 failing App.test.tsx assertions). RankingsPanel's
own resolved placeholders carry no ARIA role for the same reason -- only
the "Loading..." state announces.
@seonghobae
seonghobae enabled auto-merge (squash) August 25, 2026 11:44
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 16 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b6b16efa-d554-4a04-8623-23688d6a05d6

📥 Commits

Reviewing files that changed from the base of the PR and between 48f013a and 9a1b416.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (27)
  • AGENTS.md
  • CHANGELOG.d/2.18.0-public-ontology-namespace-completeness.md
  • CHANGELOG.md
  • backend/tests/test_api.py
  • docs/adr/0207-repository-case-ontology-namespace-canonical.md
  • docs/adr/README.md
  • docs/doctoring/ONTOLOGY_NAMESPACE_INVENTORY.md
  • docs/ontology/lineageweave-kg-shapes.ttl
  • docs/ontology/lineageweave-kg.ttl
  • docs/ontology/namespace-compatibility.ttl
  • docs/ontology/prov-o-support-profile.ttl
  • frontend/src/App.css
  • frontend/src/App.test.tsx
  • frontend/src/components/OntologyExplorer.stories.tsx
  • frontend/src/components/OntologyExplorer.tsx
  • frontend/src/components/WorkspaceCalendar.tsx
  • lineageweave/ontology.py
  • pyproject.toml
  • scripts/build_ontology_site.py
  • scripts/migrate_legacy_namespace.py
  • scripts/publish_ontology_site.py
  • tests/test_ontology.py
  • tests/test_ontology_shapes.py
  • tests/test_ontology_site.py
  • tests/test_post_chat.py
  • tests/test_prov_o.py
  • tests/test_publish_ontology_site.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Closed as a duplicate of #618, which contains the same three-file review repair on current protected main with normal auto-merge enabled. Keeping both would duplicate the ontology/fixture changes and create an avoidable conflict.

@seonghobae seonghobae closed this Aug 25, 2026
auto-merge was automatically disabled August 25, 2026 11:45

Pull request was closed

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 4 potential issues.

Open in Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Reversed migration guard reconstructs original IRI correctly

The reversed lowercase->repository-case migration rebuilds the pre-migration value via new.replace(CANONICAL_NAMESPACE, LEGACY_NAMESPACE) for the ontology_iri = $4 optimistic-lock guard. Since the replaced substring includes /ontology#, the local fragment cannot be mangled, and a re-run finds only canonical rows so it stays idempotent.

(Refers to this code)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread CHANGELOG.md
Comment on lines +11 to +15
- The repository-case public ontology namespace
`https://contextualwisdomlab.github.io/LineageWeave/ontology#` is canonical
(ADR 0207, superseding ADR 0157, resolving issue #372); the lowercase form
is a deprecated compatibility vocabulary with validated mappings and a
dry-run migration tool for stored values.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changelog still calls the lowercase namespace canonical

The new entry names the repository-case namespace canonical, but three earlier bullets in the same Unreleased section still call the lowercase namespace canonical and the repository-case form deprecated (CHANGELOG.md, CHANGELOG.md). The changelog now states the namespace roles both ways.

Prompt for agents
After ADR 0207 flips the canonical ontology namespace to the repository-case spelling, three stale bullets in the [Unreleased]/Added and Fixed sections of CHANGELOG.md still describe the old lowercase-canonical direction and now contradict the new entry at the top. Update or remove: the bullet at CHANGELOG.md:74-75 ('publishes the deprecated repository-case compatibility vocabulary' -- repository-case is now canonical, lowercase is deprecated); the bullet at CHANGELOG.md:76-77 ('mints its product class mappings only in the canonical lowercase namespace'); and the Fixed bullet at CHANGELOG.md:128 ('uses its canonical lowercase deployed IRI'). Make them consistent with the repository-case-canonical direction.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

<h3 id={`${headingId}-observed`}>{t("Observed calendar events")}</h3>
{events.length === 0 ? (
<p className="popup-placeholder" role="status">
<p className="popup-placeholder">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: Calendar fail-closed message no longer announced

The empty-events placeholder dropped role="status", so the fail-closed copy shown when Naruon is unavailable is no longer surfaced as a live region to screen readers. Tests still pass because they query by text. Unrelated to the PR's namespace/SHACL focus.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +138 to +189
def validate_shapes_graph(shapes: Graph, canonical: Graph) -> None:
"""Reject SHACL shapes whose targets dangle outside the ontology.

A shape that targets a class absent from the canonical graph, or
constrains a path neither declared there nor one of RDF's three
reification predicates, would silently validate nothing -- the
publication boundary refuses it instead (ADR 0207 decision 10).
Only URI-valued targets and paths are checked; literal sh:path values
are not part of this contract.
"""
if not any(shapes.triples((None, RDF.type, SH.NodeShape))):
raise ValueError("SHACL shapes graph declares no sh:NodeShape")
for predicate in (SH.targetClass, SH.path):
for value in shapes.objects(None, predicate):
if (
not isinstance(value, URIRef)
or str(value).startswith(CANONICAL_NAMESPACE)
or (predicate == SH.path and value in STANDARD_SHACL_PATHS)
):
continue
kind = "targetClass" if predicate == SH.targetClass else "path"
raise ValueError(
f"SHACL {kind} target outside the canonical namespace: {value}"
)
declared_classes = {
subject
for subject in canonical.subjects(RDF.type, OWL.Class)
if isinstance(subject, URIRef)
}
# Entailed classes: anything with a subclass assertion is a class.
declared_classes.update(
subject
for subject, _ in canonical.subject_objects(RDFS.subClassOf)
if isinstance(subject, URIRef)
)
declared_properties = {
subject
for subject in canonical.subjects(RDF.type, OWL.ObjectProperty)
if isinstance(subject, URIRef)
}
declared_properties.update(
subject
for subject in canonical.subjects(RDF.type, OWL.DatatypeProperty)
if isinstance(subject, URIRef)
)
declared_properties.update(STANDARD_SHACL_PATHS)
for target in shapes.objects(None, SH.targetClass):
if target not in declared_classes:
raise ValueError(f"SHACL targetClass is not an ontology class: {target}")
for path in shapes.objects(None, SH.path):
if path not in declared_properties:
raise ValueError(f"SHACL property path is not an ontology property: {path}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📝 Info: SHACL target/path validation matches the shipped shapes

validate_shapes_graph rejects sh:targetClass/sh:path values outside the canonical namespace (bar RDF reification predicates) and dangling targets. Every targetClass in the shipped shapes is a declared or subclass-entailed class and every path is a declared property or reification predicate, so the real file passes; sh:class under sh:not is intentionally unchecked.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant