Repository navigation
feat(validation): semantic checks for ontology documents - #489
Conversation
b142bd3 to
b978201
Compare
|
The undeclared-prefix check here covers the follow-up I promised in #479 — thanks for picking it up. One adjacent gap it doesn't touch: prefixes:
foaf: "not a valid iri at all !!!"Adding Happy to send a follow-up once this lands — it would collide with your |
|
Thanks, and glad the prefix check closes the loop from #479. I reproduced the gap: I would keep it as your follow-up once this lands rather than fold it in, so this PR stays reference checks only and the two hunks do not fight. It would slot into |
b978201 to
52883bf
Compare
|
|
||
| def undeclared_qname_prefix(iri: object, prefixes: dict) -> str | None: | ||
| """Return the prefix of a QName iri that prefixes does not declare.""" | ||
| if not isinstance(iri, str) or ":" not in iri: |
There was a problem hiding this comment.
This check treats any value shaped like x:y as a QName unless it contains // or a second colon. Legal absolute IRIs such as mailto:a@b.org, tel:+1555... and urn:x match that shape, so they are reported as iri uses undeclared prefix 'mailto' and validate.py exits 1 on a valid ontology.
Could we either:
- skip the check when the prefix is a known URI scheme (
mailto,tel,urn,http,https,file, ...) or - only flag it when the prefix is not declared and the value is not a valid absolute IRI?
The current tests only cover http:// and urn:isbn:..., so please add cases for mailto: and tel: (expecting no error), plus a genuine undeclared-prefix case (expecting an error).
There was a problem hiding this comment.
Good catch, thanks. The check now treats a known URI scheme before the colon (mailto, tel, urn, did, doi and a few more) as a full IRI, case-insensitively, on top of the existing "//" and second-colon rules. The accepted cases gained urn:x, mailto:, tel: and an uppercase scheme, and there is a foaf:Order case next to them that still reports the undeclared prefix; the earlier ex:Order case covers the document with no prefixes at all. 141 validation tests pass locally.
validate.py stopped at the JSON schema for ontology documents, because every semantic check was guarded by the presence of `datasets`. An ontology that declares the same concept twice, extends a concept that does not exist, identifies itself by a relationship it does not have, gives a role to an undeclared concept, or uses a QName prefix missing from `prefixes` all came back "Validation PASSED". Add validate_ontology with the same kind of checks the core side has: - unique concept names in the ontology and unique relationship names within a concept - extends, identify_by and role references resolve to declared concepts and relationships, with the built-in concepts from ontology.md (Any, Boolean, Date, DateTime, Decimal, Float, Integer, String) included and identify_by allowed to name a supertype's relationship - a cycle check on extends - QName iri values on concepts and relationships must use a prefix declared in the top-level `prefixes` map ontology_mappings are left to the schema for now. The validation CI gains a step running examples/flights.yaml against the ontology schema, which previously only covered the TPC-DS core example. Generated-by: Claude Code
mailto:, tel: and urn:x have no "//" and only one colon, so the QName check read them as prefix:local and reported an undeclared prefix. Skip the check when the part before the colon is a known URI scheme (case-insensitive); declared prefixes and the "//" and second-colon rules are unchanged. Tests cover the accepted schemes and a prefix that is genuinely undeclared. Generated-by: Claude Code
d540a42 to
e9181b0
Compare
jbonofre
left a comment
There was a problem hiding this comment.
LGTM!
The ontology validation looks solid: the built-in concepts match ontology.md and merging extends and relationship names across duplicate concepts behaves as the tests expect.
I rebased from main and resolved the conflict in test_validate.py.
Two minor notes:
find_extends_cycles.visitis recursive, so anextendschain deeper than about 1000 concepts would raiseRecursionErrorinstead of reporting a cycle. That's unlikely in practice, so good for me.- the ontology checks only run when the earlier checks pass, so a document with both a core error and an ontology error shows them in two rounds. It looks intentional to me, so good for me too 😄
Summary
validate.pyruns unique-name, reference, arity and SQL checks on core documents, but each one is guarded bydatasets, so an ontology document only got the schema. Five defects, one at a time, each came back "Validation PASSED" on main: a concept declared twice,extendsnaming an undeclared concept,identify_bynaming a relationship the concept does not have, a role played by an undeclared concept, and a QNameiriwhose prefix is not inprefixes.This adds
validate_ontology, run after the schema when the document has anontologykey:extends,identify_byand role references resolve to declared concepts and relationships. The built-in concepts from ontology.md (Any,Boolean,Date,DateTime,Decimal,Float,Integer,String) count as declared, andidentify_bymay name a relationship declared on a supertype, which is how the reference parser resolves it tooextendsirion a concept or relationship must use a prefix from the top-levelprefixesmap. Aniriwith//after the scheme or a second colon (http://...,urn:isbn:...) is read as a full IRIThe built-ins, the cycle check and the CI step are the three additions suggested on the dev@ thread.
ontology_mappingsare left to the schema until #458 settles where they live. One divergence to flag: the parser also treatsAnyEntityas a built-in, which ontology.md does not list, so I went with the spec.Testing
validation/tests: 106 passed, running the workflow's steps locally on Python 3.11 to 3.14. 16 new tests cover each check, the built-ins, an inheritedidentify_by, cycles, full IRIs, malformed shapes and the CLI path.examples/flights.yaml, the converter'sflights.yamlfixture and its round-trip snapshot all pass. The standaloneflights.ontology.yamlproposed in Ontology: decouple Ontologies, Semantic Models, and their Mappings #458 passes as well.The validation CI gains a step running
examples/flights.yamlagainst the ontology schema. It previously only validated the TPC-DS core example.Written with LLM assistance; I ran every check above myself and read the diff against the neighbouring functions.
Related Issues
Closes #488. dev@ thread: https://lists.apache.org/thread/lltdz30fryypzgvgc0ldd2tbbbhrqt10
Checklist
Validation
validation/are updated if the spec changedTests
pytest/ CI green)Compliance