Feat(ontology): add Ossie -> RelationalAI converter with formula parsing and reasoner

[vmihalovski]

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). A ply lexer 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 in OssieParser; 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 an OssieOntology into a PyRel model — concepts, relationships, entity and link mappings, derived rules. Table resolution is pluggable through a table_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 to vendor/, so vendor-specific code sits under one name.
• spec_to_ossie: a bare expression: 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 is StoreNr, not Store. 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_primitive walks extends iteratively, so a malformed cyclic hierarchy returns an answer instead of RecursionError.
• Public API gains OntologyReasoner, FormulaParserFactory, MappingFormulaParserFactory.

Packaging

relationalai is 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 from ossie_ontology/__init__.py imports it. tests/test_optional_extra.py asserts that boundary in a subprocess with the SDK masked.

Tests

199 tests, organised to mirror src/:

package	tests	covers
palantir/	54	reading an export; what the converter admits
spec/	37	parsing, and the corpus pinned by snapshot
common/	33	graph sort, text helpers
model/	26	concepts, roles, verbalizations
expr/	23	formula grammar and validation
relationalai/	12	conversion, PyRel snapshots, executability
top level	14	suite and packaging guarantees

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.yaml pins relationalai to an offline profile — required, since it will not construct a Model without a config and otherwise searches ~/.rai, ~/.snowflake and ~/.dbt. tests/test_offline.py asserts the pin, that the config carries no reachable account, and that the guard itself works.

PyRel snapshots are .py and are executed by test_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 matches test_*.py) and [tool.pyright] excludes tests/snapshots.

[Copilot]

🟡 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 OntologyReasoner and 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.

[Copilot]

🔵 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__ is None), and it bypasses Python’s normal hash() behavior. Hashing the args directly is both simpler and more correct.
  converters/ontology/tests/conftest.py:166
• The spec_model fixture docstring says formulas “stay as raw text” and that parsing would pull in the optional relationalai extra, but the fixture calls OssieParser() with default factories (which now parse/validate formulas) and formula parsing does not depend on the relationalai extra. This mismatch is likely to confuse future maintainers.

• Files reviewed: 87/105 changed files
• Comments generated: 0 new
• Review effort level: Lite

[vmihalovski]

🔵 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__ is None), and it bypasses Python’s normal hash() behavior. Hashing the args directly is both simpler and more correct.
  converters/ontology/tests/conftest.py:166

• The spec_model fixture docstring says formulas “stay as raw text” and that parsing would pull in the optional relationalai extra, but the fixture calls OssieParser() with default factories (which now parse/validate formulas) and formula parsing does not depend on the relationalai extra. This mismatch is likely to confuse future maintainers.

• Files reviewed: 87/105 changed files

• Comments generated: 0 new

• Review effort level: Lite

Fixed

[Copilot]

🔵 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

[Ostrzyciel]

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?

[vmihalovski]

@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.

[Ostrzyciel]

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.

[Ostrzyciel]

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.

[vmihalovski]

@Ostrzyciel, this is a good point. I will update the README file and add links and additional information about RelationalAI.

[jbonofre]

Thanks for this PR!

The only "blocker" from my side is in visitor/converter.py about dataset().name calling a property that returns a string, so any mapping formula with a bare dataset column fails with TypeError (same on parser.py).

[jbonofre]

dataset.name() is a @property (model.py), so dataset.name() calls a str and raises TypeError. Every mapping formula that references a bare dataset column (e.g. price * qty) goes through visit_datasetfieldhandle and will fail here. visit_dotjoinhandle a few lines down uses dataset.name without parentheses, so the two visitors disagree. The assert message on line 318 has the same bug.

Please drop the parentheses on both lines, and add a test that converts a formula with a bare column reference.

[vmihalovski]

Fixed in 4374e5b: dropped the parentheses on both lines. Added test_a_bare_column_mapping_formula_converts (in tests/relationalai/test_converter.py), which runs cashReceived - change through MappingFormulaConverter. It fails with this TypeError without the fix.

[jbonofre]

Same issue: mapping_dataset is a Dataset (it has .field()), and Dataset.name is a property, so mapping_dataset.name() raises TypeError. A typo column name then produces an opaque TypeError instead of the intended "'x' is not a field in dataset ..." message. A test for the unknown-column case would catch it.

[vmihalovski]

Fixed in 4374e5b. Added test_unknown_bare_column_names_the_dataset in tests/expr/test_parsing.py: a misspelt bare column now raises "'cashRecieved' is not a field in dataset 'PAYMENTS' and is not a known concept, relationship, or dataset" instead of TypeError.

[jbonofre]

OntologyModel._last_role_player(self, r) takes a required argument, but it's called here with none. bind(handle, **identifier) without an expr will raise TypeError. It also isn't passed the relationship, so it couldn't select the right role even with an argument.

Could you pass the relationship being bound, and cover the no-expr path with a test?

[vmihalovski]

Fixed in 4374e5b: bind now passes the relationship it is binding to _resolve_extra_key, which passes it on to _last_role_player(r). Added test_binding_by_identifier_alone_looks_up_the_last_role, which calls bind(sale.soldAt, nr=...) with no expr and checks that it emits Store.lookup(nr=...) for the last role.

[jbonofre]

The docstring says "everything the identifier is derived from goes into the key", but the referents key uses only rm.relationship.name and _expression_key(rm.expression). As far as I can tell, a referent with nested referent_mappings has expression=None and keys as the same ("other", "None") regardless of its nesting. Two object mappings of the same concept with the same top-level relationship but different nested referents would then get the same key. The second would reuse the first's EBinding, and its entities would silently never be created or bound.

Should the key recurse into nested referent mappings? A test with two such mappings would show whether they collide.

[vmihalovski]

Good catch, they did collide. The key now recurses into nested referent_mappings through a new _referents_key helper, the same way _build_entity_kwargs does (4374e5b).

test_object_mappings_differing_only_in_nested_referents_both_create covers it: Desk is identified by an Employee, and it's mapped twice from two employee-number columns. With the old key the second mapping reused the first binding, so only one Desk.new rule was emitted and the second column's Desks were silently never created. Now there's one rule per column.

[jbonofre]

When a relationship name is reserved, this warns and (below) skips it, so it never reaches properties_index. But properties_index[...] is indexed directly later. If any identifier or link mapping uses a skipped relationship, I would expect a KeyError rather than the "warn and skip" the warning implies.

Could those lookups use .get() and skip with a warning too?

[vmihalovski]

Done in 4374e5b: the lookups now go through a _lookup_property helper that warns and returns None, and each caller skips. Two choices you may want to weigh in on:

• identify_by: if any part of it was skipped, the whole declaration is dropped. Keeping only part of it would silently change what makes the entity unique.
• Link mappings: a skipped link mapping also skips its children, because they're bound through it. A skipped root unary relationship still lets its children convert.

Covered by test_mappings_over_a_reserved_relationship_are_skipped_with_a_warning.

[vmihalovski]

@jbonofre Thank you for your review. I’ll address all the items and let you know once it’s done so you can do another round of review.

[vmihalovski]

@jbonofre I’ve addressed all your comments. Would you mind taking another look when you have a chance?
Thank you in advance!

[jbonofre]

@vmihalovski Great, thanks! I will do a new pass.

[jbonofre]

t_INTEGER = \d+(?!\.\d) backtracks on multi-digit floats. For 31.4, the regex matches 3 because the next char is 1, not .digit. FLOAT then matches 1.4, so the lexer emits INTEGER(3) FLOAT(1.4). 31.4 + 10.5 lexes to INTEGER 3, FLOAT 1.4, PLUS, INTEGER 1, FLOAT 0.5. Any literal with two or more integer digits before the point (10.5, 100.0) fails to parse, or parses wrongly.

I suggest defining FLOAT before INTEGER and dropping the lookahead, or using \d+(?![\d.]). Could you add a lexer test for 10.5, 100.0 and 31.4 + 10.5?

[vmihalovski]

The fix has been applied, and a test has been added.

[jbonofre]

expr: MINUS expr %prec NOT gives unary minus precedence of NOT, which is below PLUS/MINUS/TIMES/DIVIDE. The parser shifts on the following operator instead of reducing, so:

• -1 + 3 parses as 0 - (1 + 3) = -4 (it should be 2)
• -1 -3 parses as 0 - (1 - 3) = 2 (it should be -4)

This is silent and flows into the generated PyRel, so something like -discount + tax yields wrong values with no error.

I suggest a dedicated UMINUS precedence entry above TIMES/DIVIDE and %prec UMINUS on this rule. Please add tests for -a + b, -a - b and -a * b.

[vmihalovski]

The fix has been applied, and a test has been added.

[jbonofre]

_expression_type now pins the field type from the container's identifying type even when the object mapping names no concept:, and _pin_field_type compares with is not. A dataset column used in two mappings whose identifying types differ (including subtype vs supertype) will now raise "A dataset field can only be bound to one ontology concept type". Specs that were accepted before this PR would be rejected.

Is this intentional? If so, a note in the PR description would help. If not, comparing with subtype compatibility rather than identity may be more appropriate. It's more a question for you 😄

[vmihalovski]

Rather than switch to subtype-compatible comparison, I've removed the pinning altogether:

• Mappings no longer set a column's type. A field's type now comes only from the core spec's own datatype attribute, which we previously ignored. It's read into the model, written back by ossie_to_spec, and set by the Palantir converter. A column with no datatype stays untyped, so one column can feed any number of value types, and the "can only be bound to one ontology concept type" error is gone.
• Mappings are checked against datatype when it's present. Each mapping that reads a column checks the column's datatype against the base type of what it reads the column as:
  • with concept:, that concept (an entity's key type if it's an entity)
  • without concept:, the container's key type, or the role player for referent mappings

So an Integer column can key a StoreNr extends Integer, but a String one can't. Concept: doesn't skip the check; even a cast can't make a String column hold Integers. If there's no datatype (or it's Opaque), we don't check anything, so specs without datatype behave as before. DateTimeTz counts as DateTime.

• Explicit casts in expressions: StoreNr(T.id) reads the column as a given value type and is emitted as the same cast in PyRel. The wrapper must be the expected type or a subtype of it (via OntologyReasoner.in_subtype_closure), and the column must still have the wrapper's base type.

[vmihalovski]

@jbonofre, thank you again for your review! I’ve addressed your comments and replied to each of them. Would you mind taking another look?

Thank you in advance!

[kayemkim]

The RelationalAI tests in this PR never run in CI. converter-ontology-ci.yml does a plain uv sync, so relationalai is not installed and tests/relationalai/test_converter.py skips itself at import; every upstream run of this branch shows that as 1 skipped. With --extra relationalai they pass offline, using the config in tests/config/raiconfig.yaml, so a second sync step or a matrix leg would let CI cover the PyRel snapshots and the executable-model check.

The formula lexer only accepts ASCII identifiers: t_ID is [a-zA-Z_0-9]+. Concept names elsewhere are not restricted that way. A copy of examples/flights.yaml with CancelationCode renamed to CancelationCodé passes validate.py against the ontology schema and loads with the current OssieParser, but on this branch it stops with SyntaxError: Illegal character "é == 'A' OR ...". Since formula parsing is now the default path, a document that loads today becomes a hard failure. Changing the rule to \w+ makes the copy parse and leaves the existing suite green. While there, t_error prints the whole remaining input as the illegal character; t.value[0] would be the usual PLY idiom.

Dependency Review is failing on multidict@6.8.0 in converters/ontology/uv.lock (GHSA-54p9-h82j-f925, pulled in through the extra). uv lock --upgrade-package multidict moves it to 6.9.1 with no other changes.

One question on the examples: eight fields in flights stay without datatype (cancelled, diverted, air_time, ...), and per the new test_a_spec_without_datatypes_declares_string_columns those become String columns in PyRel. Is that the intended shape for cancelled and diverted, or are they just not known well enough to call Boolean?

[vmihalovski]

@kayemkim Thanks for the careful review. All of it is addressed in the latest push:

Non-ASCII identifiers. t_ID now matches \w+, so CancelationCodé lexes as an ID. Your renamed copy of flights.yaml now loads through OssieParser, and the existing suite stays green. I also switched t_error to report t.value[0], so the message names only the offending character. Both cases have tests in tests/expr/test_lexer.py (64d8090).

multidict. I ran uv lock --upgrade-package multidict, which moved it from 6.8.0 to 6.9.1. Only that package changed in uv.lock (be123f2).

Untyped flights fields. Good catch. cancelled and diverted were always meant to be Boolean, since the mappings compare both to TRUE. Without a datatype, the generated PyRel declared them as String and then compared them to True. Both are now datatype: Boolean in examples/flights.yaml, examples/flights.semantic_model.yaml and the vendored fixture, with the snapshots refreshed (cb0d98b). The remaining six (air_time, duration, wheels_on, wheels_off, scheduled_duration and AIRPORT.opened) simply haven't been typed yet, so for now they stay String.

RelationalAI tests in CI. You're right, they were skipped on every run. converter-ontology-ci.yml still runs the base suite after a plain uv sync, which keeps the no-extra install covered. After that it runs uv sync --extra relationalai and uv run pytest tests/relationalai. Locally, with uv 0.12.19, the 18 RelationalAI tests pass offline on 3.11–3.14 against tests/config/raiconfig.yaml (0601c15).

[jbonofre]

FormulaParserFactory caches its FormulaParser and OntologyReasoner under id(ontology). MappingFormulaParserFactory uses (id(ontology, id(semantic_model)). These factories are now the defaults for OssieParser and the converters, so they outlive individual conversions. CPython can reuse the id of a garbage-collected object. A later ontology could then get a parser and reasoner bound to the dead one, and its formulas would resolve against the wrong concepts and relationships.

Could we key the cache on the ontology object itself, or hold a weak reference? A test that converts two ontologies with gc.collect() between them would cover this.

[vmihalovski]

Good point, thanks. In 4fe1fed the caches are keyed on the ontology and semantic model objects instead of their id(). As it happens, the cached parser already held a strong reference to the ontology, so the id could not be reused while the cache entry existed. But that was a side effect rather than a design choice, so it is better to make it explicit. I added a test that parses two ontologies with gc.collect() in between.

[jbonofre]

Role.name is the explicit role name (for example seller), not the concept name. If the role's variable isn't in _var_refs, lookup_concept("seller") returns None and the assertion fires.

FormulaValidator.validate_rule exempts identifier relationships for needing head vars in the body. A derived_by on an identifier relationship, or on identify_by, with an explicitly named role that the body never mentions, therefore validates and then aborts the whole conversion.

Should the fallback use role.player.name?

[vmihalovski]

Fixed in 4fe1fed: the fallback now uses role.player.name, with a test for a named role the body never mentions (test_a_named_role_the_body_never_mentions_falls_back_to_its_player).

[jbonofre]

visit_disjunction and visit_whereexpr are handled explicitly here, but visit_exists is not. FormulaConverter and MappingFormulaConverter inherit GenericVisitor.visit_exists, which is @abtractmethod with a pass body.
GenericVisitor is not an ABC, so nothing enforces the override. Exists.accept therefore returns None and _flatten_formula drops it. A derived_by or mapping formula containing EXISTS(...) converts without the EXISTS condition and without an error.

Could we implement it, or raise NotImplementedError like the two neighbouring visitors?

[vmihalovski]

Fixed in 4fe1fed: EXISTS now raises (NotImplementedError in rules, ValueError in mappings) instead of being dropped, with a test. I also made GenericVisitor an ABC, so a missing handler fails at instantiation.

[vmihalovski]

@jbonofre, thank you again for your review! I’ve addressed your comments and replied to each of them. Would you mind taking another look?

Thank you in advance!