Feat(ontology): add Ossie -> RelationalAI converter with formula parsing and reasoner - #412
vmihalovski wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed correctness issues in newly added core modules (formula lexer error handling, operator classification, and RelationalAI role root handling) that should be fixed before approval.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds an Ossie → RelationalAI (PyRel) conversion path and introduces a first-class formula parsing/validation layer plus an ontology reasoner to support converters and expression validation, with substantial test/fixture coverage to keep behavior pinned and the relationalai SDK optional.
Changes:
- Added
OntologyReasonerand extended converters to use it (e.g., reference schemes, subtype closure) and to correctly type mapping expressions for entity identifiers. - Made formula parsing/validation the default (PLY-based lexer/parser + validators/visitors + factories), with an opt-out to keep formulas as raw text.
- Expanded the test suite and pinned fixtures/snapshots to enforce offline operation, optional-extra boundaries, and converter/parser behavior.
File summaries
| File | Description |
|---|---|
| converters/ontology/tests/test_reasoner.py | New unit tests covering OntologyReasoner behaviors (ref schemes, subtype closure, cycle handling). |
| converters/ontology/tests/test_optional_extra.py | Subprocess-based guard ensuring relationalai remains an optional extra and doesn’t leak into base imports. |
| converters/ontology/tests/test_offline.py | Tests enforcing offline-only execution (pinned config + network guard behavior). |
| converters/ontology/tests/test_examples_in_sync.py | Updates vendored spec path to match new fixtures layout. |
| converters/ontology/tests/spec/test_parser.py | Adds regression tests for mapping field typing under bare-expression entity mappings and cyclic-extends primitive checks. |
| converters/ontology/tests/spec/test_corpus.py | Expands snapshot coverage from a single spec to the full spec corpus (structure + roundtrip YAML). |
| converters/ontology/tests/spec/snapshots/test_corpus/test_structure_snapshot/tpch/structure.txt | New pinned structure snapshot for tpch. |
| converters/ontology/tests/spec/snapshots/test_corpus/test_structure_snapshot/retail/structure.txt | New pinned structure snapshot for retail. |
| converters/ontology/tests/spec/snapshots/test_corpus/test_structure_snapshot/flights/structure.txt | New pinned structure snapshot for flights. |
| converters/ontology/tests/spec/init.py | Declares the spec-test package. |
| converters/ontology/tests/relationalai/snapshots/test_converter/test_palantir_export_converts_all_the_way_to_pyrel/palantir_chain.py | New PyRel snapshot for Palantir chain conversion. |
| converters/ontology/tests/relationalai/init.py | Declares the RelationalAI test package and documents offline expectations. |
| converters/ontology/tests/palantir/test_parser.py | Updates Palantir imports to the new ossie_ontology.vendor.palantir namespace. |
| converters/ontology/tests/palantir/converter/test_relation_statuses.py | New tests for relation-status conversion policies and collision behavior. |
| converters/ontology/tests/palantir/converter/test_property_statuses.py | New tests for property-status conversion/mapping policies and subclass overrides. |
| converters/ontology/tests/palantir/converter/test_primary_keys.py | New tests for PK column matching, ordering stability, and warnings on missing columns. |
| converters/ontology/tests/palantir/converter/test_object_type_statuses.py | New tests for object-type admission policy, builtin-name collision handling, and subclass overrides. |
| converters/ontology/tests/palantir/converter/test_multi_dataset_links.py | New tests for correct link attachment when object types map to multiple datasets. |
| converters/ontology/tests/palantir/converter/test_many_to_many.py | New tests for M:M/intermediary relations and derived-by behavior, including collision skipping. |
| converters/ontology/tests/palantir/converter/test_inheritance.py | New tests for when PK-to-PK relations become inheritance and for cycle-breaking behavior. |
| converters/ontology/tests/palantir/converter/helpers.py | New shared helper utilities for building/reading converted Palantir exports in tests. |
| converters/ontology/tests/palantir/converter/builders.py | New builders for constructing Palantir export JSON fixtures in a single place. |
| converters/ontology/tests/palantir/converter/init.py | Declares the Palantir converter-test package and documents policy dimensions. |
| converters/ontology/tests/palantir/init.py | Declares the Palantir test package and documents parser vs converter suite responsibilities. |
| converters/ontology/tests/model/test_verbalization.py | New tests pinning verbalization parsing semantics (prefix/postfix rules, builtin-name tokens). |
| converters/ontology/tests/model/test_roles.py | New tests ensuring Role identity semantics (especially unnamed self-relationships). |
| converters/ontology/tests/model/test_concept_lookup.py | New tests pinning lookup_concept vs ensure_builtin_concept behaviors. |
| converters/ontology/tests/model/init.py | Declares the model-test package. |
| converters/ontology/tests/fixtures/validation/unresolved_var_type.yaml | New validation fixture: unresolved var reference in rule formula. |
| converters/ontology/tests/fixtures/validation/undefined_relationship.yaml | New validation fixture: undefined relationship reference. |
| converters/ontology/tests/fixtures/validation/top_level_require_valid.yaml | New validation fixture: valid ontology-level requires. |
| converters/ontology/tests/fixtures/validation/top_level_require_unknown_concept.yaml | New validation fixture: unknown concept in ontology-level aggregate. |
| converters/ontology/tests/fixtures/validation/top_level_require_free_var.yaml | New validation fixture: illegal free var in ontology-level requires. |
| converters/ontology/tests/fixtures/validation/top_level_require_exists_valid.yaml | New validation fixture: ontology-level free var legalized by EXISTS. |
| converters/ontology/tests/fixtures/validation/signature_mismatch.yaml | New validation fixture: signature mismatch across relationship references. |
| converters/ontology/tests/fixtures/validation/scoped_require_scope_valid.yaml | New validation fixture: complex scope-binding rules (facts, EXISTS, aggregates, traversal). |
| converters/ontology/tests/fixtures/validation/require_sub_exists_valid.yaml | New validation fixture: EXISTS scoping inside concept/relationship requires. |
| converters/ontology/tests/fixtures/validation/require_sub_exists_unknown_rel.yaml | New validation fixture: unknown relationship referenced under EXISTS. |
| converters/ontology/tests/fixtures/validation/not_declared_concept_in_relationship.yaml | New validation fixture: role concept not declared in ontology. |
| converters/ontology/tests/fixtures/validation/not_declared_concept_in_identifier.yaml | New validation fixture: identify_by references unknown relationship. |
| converters/ontology/tests/fixtures/validation/mapping_formula_valid.yaml | New validation fixture: valid mapping expression formula. |
| converters/ontology/tests/fixtures/validation/mapping_formula_unknown_field.yaml | New validation fixture: mapping formula referencing unknown dataset column. |
| converters/ontology/tests/fixtures/validation/mapping_formula_concept_ref.yaml | New validation fixture: mapping formula forbidden concept reference. |
| converters/ontology/tests/fixtures/validation/mapping_formula_agg_with_rel.yaml | New validation fixture: mapping formula forbidden relationship reference. |
| converters/ontology/tests/fixtures/validation/bare_field_in_formula.yaml | New validation fixture: bare field/var in formula should be rejected. |
| converters/ontology/tests/fixtures/validation/ambiguous_type_vars.yaml | New validation fixture: ambiguous type inference for vars. |
| converters/ontology/tests/fixtures/palantir/retail_mini/ontology.json | New minimal Palantir export fixture for converter/parser tests. |
| converters/ontology/tests/fixtures/palantir/retail_mini/data_sets/ds.json | New dataset fixture accompanying the minimal Palantir export. |
| converters/ontology/tests/fixtures/behaviours/ref_scheme.yaml | New behavior fixture exercising ref-scheme-driven conversion branches. |
| converters/ontology/tests/fixtures/behaviours/key_column_both_ways.yaml | New behavior fixture ensuring key columns are typed consistently across mapping styles. |
| converters/ontology/tests/fixtures/behaviours/derived_identifier.yaml | New behavior fixture for derived identifiers ensuring correct PyRel head emission. |
| converters/ontology/tests/expr/test_validator.py | New tests directly exercising validator dot-join container type checks. |
| converters/ontology/tests/expr/test_parsing.py | New end-to-end tests for formula parsing/validation defaults and opt-out behavior. |
| converters/ontology/tests/expr/init.py | Declares the expression-test package. |
| converters/ontology/tests/conftest.py | Restructures fixture dirs, pins offline RelationalAI config, adds network deny fixture, and adds corpus param fixtures. |
| converters/ontology/tests/config/raiconfig.yaml | Adds an explicit offline RelationalAI config for tests. |
| converters/ontology/tests/common/test_text_utils.py | New tests pinning name normalization utilities and warning behavior. |
| converters/ontology/tests/common/test_graph.py | New tests for topological sort behavior, including unknown nodes vs real cycles. |
| converters/ontology/tests/common/init.py | Declares the common-test package. |
| converters/ontology/src/ossie_ontology/vendor/relationalai/roles.py | Adds PyRel FieldRef role helper utilities for the RelationalAI side. |
| converters/ontology/src/ossie_ontology/vendor/relationalai/init.py | Declares the RelationalAI vendor package. |
| converters/ontology/src/ossie_ontology/vendor/palantir/parser/init.py | Updates Palantir parser imports to the new vendor namespace. |
| converters/ontology/src/ossie_ontology/vendor/palantir/init.py | Declares the Palantir vendor package. |
| converters/ontology/src/ossie_ontology/vendor/init.py | Introduces a unified vendor/ namespace for spoke models (Palantir, RelationalAI). |
| converters/ontology/src/ossie_ontology/reasoner.py | Introduces OntologyReasoner for ref schemes, identifier-relationship recognition, and subtype-closure checks. |
| converters/ontology/src/ossie_ontology/parser/init.py | Switches default factories to parsed-formula factories (with opt-out to raw factories). |
| converters/ontology/src/ossie_ontology/model.py | Makes Concept.is_primitive iterative and cycle-safe. |
| converters/ontology/src/ossie_ontology/expr/model.py | Adds expression AST node types (BinOp/Expression) used by formula parsing/validation. |
| converters/ontology/src/ossie_ontology/expr/formula/visitor/unbound.py | Adds unbound-reference analysis to enforce binder/scope rules. |
| converters/ontology/src/ossie_ontology/expr/formula/visitor/collector.py | Adds handle collection + type inference to support validation and later conversion. |
| converters/ontology/src/ossie_ontology/expr/formula/visitor/init.py | Adds base visitors and shared operator application utilities. |
| converters/ontology/src/ossie_ontology/expr/formula/lexer.py | Adds PLY lexer for formula syntax. |
| converters/ontology/src/ossie_ontology/expr/formula/init.py | Declares the formula parsing package. |
| converters/ontology/src/ossie_ontology/expr/factory.py | Adds parsing factories wiring lexer/parser/validators into model formula hooks. |
| converters/ontology/src/ossie_ontology/expr/common.py | Adds shared enums/utilities for expression parsing (aggregation methods). |
| converters/ontology/src/ossie_ontology/expr/init.py | Adds base AST node protocol + acceptor decorator. |
| converters/ontology/src/ossie_ontology/converter/spec_to_ossie/converter.py | Updates mapping-expression typing for entity identifiers using reasoner ref schemes. |
| converters/ontology/src/ossie_ontology/converter/palantir_to_ossie/converter.py | Updates Palantir model imports and defaults formula parsing behavior. |
| converters/ontology/src/ossie_ontology/converter/ossie_to_relationalai/init.py | Adds the RelationalAI converter API surface (kept behind the optional extra). |
| converters/ontology/src/ossie_ontology/init.py | Exposes reasoner and formula parsing factories; updates Palantir parser import path. |
| converters/ontology/scripts/palantir_to_ossie.py | Updates Palantir parser import path to the vendor namespace. |
| converters/ontology/README.md | Documents the RelationalAI converter, the optional extra, and formula parsing defaults/opt-out. |
| converters/ontology/pyproject.toml | Adds ply dependency, defines relationalai optional extra, and updates pyright excludes. |
| converters/ontology/.gitignore | Minor formatting correction. |
Review details
- Files reviewed: 87/105 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🔵 Needs a closer look
There are confirmed runtime-correctness issues in newly added code (e.g., role_part_of not handling Reading roots and an unsafe Expression.__hash__) that should be fixed before approval.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
converters/ontology/src/ossie_ontology/expr/model.py:70
Expression.__hash__calls each argument’s__hash__()directly. For unhashable args this can raise a confusing'NoneType' object is not callable(because__hash__isNone), and it bypasses Python’s normalhash()behavior. Hashing the args directly is both simpler and more correct.
converters/ontology/tests/conftest.py:166- The
spec_modelfixture docstring says formulas “stay as raw text” and that parsing would pull in the optionalrelationalaiextra, but the fixture callsOssieParser()with default factories (which now parse/validate formulas) and formula parsing does not depend on therelationalaiextra. This mismatch is likely to confuse future maintainers.
- Files reviewed: 87/105 changed files
- Comments generated: 0 new
- Review effort level: Lite
Fixed |
There was a problem hiding this comment.
🔵 Needs a closer look
The new vendor/relationalai/roles.py helper functions contradict their own stated root-type support (Reading) and will raise or mis-handle role sibling traversal in valid root configurations.
Review details
- Files reviewed: 87/105 changed files
- Comments generated: 0 new
- Review effort level: Lite
|
|
||
| import ply.lex as lex | ||
|
|
||
| class FormulaLexer: |
There was a problem hiding this comment.
You state that this is a "lexer and parser for Ossie's formula grammar" – but where is the formula grammar defined? I can't see it here: https://github.com/apache/ossie/blob/main/ontology/ontology.md
I'd be happy to implement it as well, but I can't implement something that does not exist. What is the plan to make other converters compatible with this?
There was a problem hiding this comment.
@Ostrzyciel, the Ontology working group was reviewing the formula language for a few months, and you can find examples of it in the flights.yaml as well as in the ontology.md the grammar that defines this language can be found here.
You may not have noticed this because we were discussing it in the Ontology working group, and I see that you only recently joined this group.
The semantics of relational formulas are context-specific. A discussion of this is in sections 4 and 5 of the draft Ossie Ontology Semantic document.
There was a problem hiding this comment.
OK, thank you for the links. But, as these documents are not in the repository, I assume these are still drafts that were not yet accepted into the spec. Shall we perhaps first merge them into the repository? Then it would be clear which spec version is the parser targeting, and what is correct and what is not. This is not really possible to do with an unversioned Google Doc. The doc could be changed tomorrow and we wouldn't know.
| # imports it — so it stays out of the base install. | ||
| [project.optional-dependencies] | ||
| relationalai = [ | ||
| "relationalai==1.27.1", |
There was a problem hiding this comment.
This package is non-free. This may require PMC approval (please see the PR checklist that you get when you open a new PR).
In my opinion, at the very least it should be clearly stated in the README that this is a fully proprietary package and what is its license (the PyPI listing does not say that). That matters because the license can be arbitrarily restrictive, so people installing this may unknowingly get into trouble.
Disclaimer: I'm not a lawyer, nor a PMC member.
There was a problem hiding this comment.
@Ostrzyciel, this is a good point. I will update the README file and add links and additional information about RelationalAI.
Summary
Adds an Ossie -> RelationalAI converter: an Ossie ontology converts into an executable PyRel model.
What's added
expr/— the formula layer (~2,550 lines). Aplylexer and parser for Ossie's formula grammar, an AST, a validator, and visitors that collect handles, resolve unbound references, and emit PyRel. Parsing is on by default inOssieParser; the raw string factories remain as an opt-out for sources whose formula vocabulary this grammar doesn't model.converter/ossie_to_relationalai/(~655 lines). Converts anOssieOntologyinto a PyRel model — concepts, relationships, entity and link mappings, derived rules. Table resolution is pluggable through atable_provider, so a caller can read from somewhere other than a warehouse without the converter knowing such alternatives exist.vendor/relationalai/(~2,280 lines).OntologyModel, the binding types the converter emits against, and role helpers.reasoner.py(159 lines). Answers which relationships identify a concept (ref_scheme), whether a relationship constructs one (is_identifier_relationship), and how concepts sit in the type hierarchy (in_subtype_closure). Only identifiers are indexed; everything else is read off the model on demand, so the answers don't depend on the order concepts were registered.What's changed
external/renamed tovendor/, so vendor-specific code sits under one name.spec_to_ossie: a bareexpression:mapping on an entity now takes its type from the entity's reference scheme rather than from the entity itself — a column of store numbers isStoreNr, notStore. This also removes a spurious conflict when one column was keyed through both mapping styles.ossie_to_relationalai: a bare-expression link node with children keys its child binding by the identifier value rather than the wrapped entity.model.py:Concept.is_primitivewalksextendsiteratively, so a malformed cyclic hierarchy returns an answer instead ofRecursionError.OntologyReasoner,FormulaParserFactory,MappingFormulaParserFactory.Packaging
relationalaiis an opt-in extra (pip install "apache-ossie-ontology[relationalai]"). Parsing Ossie, converting Palantir and reading or writing the spec all work without it, and nothing reachable fromossie_ontology/__init__.pyimports it.tests/test_optional_extra.pyasserts that boundary in a subprocess with the SDK masked.Tests
199 tests, organised to mirror
src/:palantir/spec/common/model/expr/relationalai/The suite runs offline and enforces it. An autouse fixture refuses every socket connection, so a test that reaches for a warehouse fails immediately rather than succeeding on whichever machine has credentials.
tests/config/raiconfig.yamlpinsrelationalaito an offline profile — required, since it will not construct a Model without a config and otherwise searches~/.rai,~/.snowflakeand~/.dbt.tests/test_offline.pyasserts the pin, that the config carries no reachable account, and that the guard itself works.PyRel snapshots are
.pyand are executed bytest_generated_pyrel_is_executable, so what's asserted is the code a user would otherwise have hand-written. pytest doesn't collect them (it only matchestest_*.py) and[tool.pyright]excludestests/snapshots.