Conversation
jbonofre
left a comment
There was a problem hiding this comment.
Thanks for this proposal @shao-xie ! Decoupling the ontology, semantic model, and mapping into separate standalone documents is a great design that directly aligns with the core spec's "one semantic model per document" direction and enables clean reuse without duplicating models.
Before we can merge, please address the following items:
- Rebase on
mainconsidering merge conflictsontology/ontology.md. - Fix schema resolution in
validation/validate.py. - If possible, add unit tests in
test_ontology.py. - Update the CI (
validation-ci.yml) update the path triggers to includeexamples/flights*.yamland add validation steps for the new examples so CI guards against regressions. - Spec checklist as I was confused 😄 In the PR description you checked
Spec changes are included in core-specbut no changes incore-specare included. You should not check this point.
| "concept_mappings": { | ||
| "type": "array", | ||
| "items": { | ||
| "$ref": "https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping" | ||
| }, | ||
| "description": "Maps logical model constructs to some concept and its relationships in the referenced ontology" | ||
| }, |
There was a problem hiding this comment.
Because this $ref points to https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping, validating a mapping document offline or via validation/validate.py currently fails:
[Schema] Cannot resolve schema reference: https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping
In validation/validate.py, validate_schema only pre-registers core-spec/ossie-schema.json in its referencing.Registry. To allow flights.mapping.yaml (and future mapping documents) to validate, please update validate_schema in validation/validate.py to also load and register ontology/ontology.json (under both its $id and its raw GitHub URL), e.g.:
ontology_path = Path(__file__).parent.parent / "ontology" / "ontology.json"
ontology = json.loads(ontology_path.read_text())
ontology_resource = Resource.from_contents(ontology)
registry = Registry().with_resources([
(core["$id"], resource),
("https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json", resource),
(ontology["$id"], ontology_resource),
("https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json", ontology_resource),
])
| "type": "string", | ||
| "description": "Must equal the referenced document's own 'name'; carries this reference's checkable identity regardless of location" | ||
| }, | ||
| "iri": { |
There was a problem hiding this comment.
Notice that iri is optional here (required: ["name"]), which makes sense for environments where a catalog resolves document locations purely by logical name.
In ontology/ontology.md, it would be helpful to explicitly document this distinction in a small sub-table (stating that name is required and iri is optional) to avoid ambiguity.
| | `description` | string | No | Human-readable description | | ||
| | `ai_context` | string/object | No | Additional context for AI tools | | ||
| | `ontology` | list | Yes | Concepts and relationships they group that form this ontology | | ||
| | `ontology_mappings` | list | No | Deprecated; accepted only so existing documents continue to validate. Write mappings as [mapping documents](#mapping-documents) instead | |
There was a problem hiding this comment.
Heads up: there is a merge conflict here with recent PR #400 (which added prefixes to this table). When rebasing onto main, please ensure both prefixes and ontology_mappings rows are retained.
| in the ontology. Just as ontologies are partitioned by concept, ontology maps partition into concept | ||
| mappings that group by some concept. | ||
|
|
||
| ### Mapping documents |
There was a problem hiding this comment.
There is a merge conflict at the start of this section against PR #441 (which documented embedding complete core documents inside ontology_mappings). When rebasing, placing the ### Mapping documents section first and keeping the deprecation note for embedded mappings will align nicely.
Also, could we add a sub-table here for the reference object (DocumentReference)?
| - **0.2.0.dev0** (2026-05-29): Basic support for ontologies and logical schema mappings | ||
| - Core ontology structure: Concepts, relationships, and business rules (requires and derived_by) | ||
| - Schema mappings from one or more logical models into an ontology | ||
| - Mapping documents (`ontology/mapping.json`) that reference one ontology and one semantic |
| ontology: | ||
| name: Flights | ||
| iri: ./flights.ontology.yaml | ||
| semantic_model: | ||
| name: Flights semantic model | ||
| iri: ./flights.semantic_model.yaml |
There was a problem hiding this comment.
The names align cleanly with flights.ontology.yaml and flights.semantic_model.yaml. The relative ./ reference is clean and works well for file-based repository layouts.
Add ontology/mapping.json for mapping documents that reference exactly one
ontology and one semantic model by {name, iri} instead of embedding them, and
deprecate the embedded ontology_mappings list. Document the mapping document
and its reference object in ontology.md, and split examples/flights.yaml into
flights.ontology.yaml, flights.semantic_model.yaml and flights.mapping.yaml.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEsT5t9mcXM6fF4QUC8Y7V
Register ontology/ontology.json alongside the core schema in validate.py's registry, so mapping documents that reference ConceptMapping validate offline. Add tests for mapping documents and the split flights examples, and validate the flights examples in Validation CI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEsT5t9mcXM6fF4QUC8Y7V
843147b to
b78ab22
Compare
|
Thanks for the thorough review, JB.
I have addressed your suggestions in the PR and its description (good catch
on the core-spec checkbox). PTAL.
…On Tue, Sep 29, 2026 at 10:23 AM JB Onofré ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Thanks for this proposal @shao-xie <https://github.com/shao-xie> !
Decoupling the ontology, semantic model, and mapping into separate
standalone documents is a great design that directly aligns with the core
spec's "one semantic model per document" direction and enables clean reuse
without duplicating models.
Before we can merge, please address the following items:
1. Rebase on main considering merge conflicts ontology/ontology.md.
2. Fix schema resolution in validation/validate.py.
3. If possible, add unit tests in test_ontology.py.
4. Update the CI (validation-ci.yml) update the path triggers to
include examples/flights*.yaml and add validation steps for the new
examples so CI guards against regressions.
5. Spec checklist as I was confused 😄 In the PR description you
checked Spec changes are included in core-spec but no changes in
core-spec are included. You should not check this point.
------------------------------
In ontology/mapping.json
<#458 (comment)>:
> + "concept_mappings": {
+ "type": "array",
+ "items": {
+ "$ref": "https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping"
+ },
+ "description": "Maps logical model constructs to some concept and its relationships in the referenced ontology"
+ },
Because this $ref points to
https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping,
validating a mapping document offline or via validation/validate.py
currently fails:
[Schema] Cannot resolve schema reference: https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json#/$defs/ConceptMapping
In validation/validate.py, validate_schema only pre-registers
core-spec/ossie-schema.json in its referencing.Registry. To allow
flights.mapping.yaml (and future mapping documents) to validate, please
update validate_schema in validation/validate.py to also load and
register ontology/ontology.json (under both its $id and its raw GitHub
URL), e.g.:
ontology_path = Path(__file__).parent.parent / "ontology" / "ontology.json"
ontology = json.loads(ontology_path.read_text())
ontology_resource = Resource.from_contents(ontology)
registry = Registry().with_resources([
(core["$id"], resource),
("https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json", resource),
(ontology["$id"], ontology_resource),
("https://raw.githubusercontent.com/apache/ossie/main/ontology/ontology.json", ontology_resource),
])
------------------------------
In ontology/mapping.json
<#458 (comment)>:
> + "custom_extensions": {
+ "$ref": "https://raw.githubusercontent.com/apache/ossie/main/core-spec/ossie-schema.json#/$defs/SemanticModel/properties/custom_extensions"
+ }
+ },
+ "required": ["version", "name", "ontology", "semantic_model", "concept_mappings"],
+ "additionalProperties": false,
+ "$defs": {
+ "DocumentReference": {
+ "type": "object",
+ "description": "Reference to another Ossie document by its logical name, with an iri saying where to resolve it from",
+ "properties": {
+ "name": {
+ "type": "string",
+ "description": "Must equal the referenced document's own 'name'; carries this reference's checkable identity regardless of location"
+ },
+ "iri": {
Notice that iri is optional here (required: ["name"]), which makes sense
for environments where a catalog resolves document locations purely by
logical name.
In ontology/ontology.md, it would be helpful to explicitly document this
distinction in a small sub-table (stating that name is required and iri
is optional) to avoid ambiguity.
------------------------------
In ontology/ontology.md
<#458 (comment)>:
> @@ -88,6 +88,7 @@ hierarchically, grouping each relationship under the concept that plays its firs
| `description` | string | No | Human-readable description |
| `ai_context` | string/object | No | Additional context for AI tools |
| `ontology` | list | Yes | Concepts and relationships they group that form this ontology |
+| `ontology_mappings` | list | No | Deprecated; accepted only so existing documents continue to validate. Write mappings as [mapping documents](#mapping-documents) instead |
Heads up: there is a merge conflict here with recent PR #400
<#400> (which added prefixes to this
table). When rebasing onto main, please ensure both prefixes and
ontology_mappings rows are retained.
------------------------------
In ontology/ontology.md
<#458 (comment)>:
> @@ -385,6 +386,41 @@ Ontology mappings declare how to map the values of fields at the logical level t
in the ontology. Just as ontologies are partitioned by concept, ontology maps partition into concept
mappings that group by some concept.
+### Mapping documents
There is a merge conflict at the start of this section against PR #441
<#441> (which documented embedding
complete core documents inside ontology_mappings). When rebasing, placing
the ### Mapping documents section first and keeping the deprecation note
for embedded mappings will align nicely.
Also, could we add a sub-table here for the reference object (
DocumentReference)?
------------------------------
In ontology/ontology.md
<#458 (comment)>:
> @@ -590,6 +626,8 @@ though `Store` plays a role in three of the relationships.
- **0.2.0.dev0** (2026-05-29): Basic support for ontologies and logical schema mappings
- Core ontology structure: Concepts, relationships, and business rules (requires and derived_by)
- Schema mappings from one or more logical models into an ontology
+ - Mapping documents (`ontology/mapping.json`) that reference one ontology and one semantic
This section conflicts with commits on main from PR #400
<#400> (IRIs and prefixes) and PR #441
<#441> (embedded core document
versions). Please retain those entries when resolving the rebase.
------------------------------
In examples/flights.mapping.yaml
<#458 (comment)>:
> +ontology:
+ name: Flights
+ iri: ./flights.ontology.yaml
+semantic_model:
+ name: Flights semantic model
+ iri: ./flights.semantic_model.yaml
The names align cleanly with flights.ontology.yaml and
flights.semantic_model.yaml. The relative ./ reference is clean and works
well for file-based repository layouts.
—
Reply to this email directly, view it on GitHub
<#458?email_source=notifications&email_token=ALP4WOGLCE6X5FHYAUWT5TD5RPAVTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZVGI4TANJVGYZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-5352905563>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ALP4WOFPDZFLX3X33T3WYGD5RPAVTAVCNFSNUABGKJSXA33TNF2G64TZHMYTAOJZGM4DEMJTG45US43TOVSTWNJVG4YDKOBZGY4DBILWAI>
.
Triage notifications, keep track of coding agent tasks and review pull
requests on the go with GitHub Mobile for iOS
<https://github.com/notifications/mobile/ios/ALP4WOBAE25M2HMUV2CRD6L5RPAVTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZVGI4TANJVGYZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJKTGN5XXIZLSL5UW64Y>
and Android
<https://github.com/notifications/mobile/android/ALP4WOBIRYNJKQYLOXBQ3IT5RPAVTA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTKMZVGI4TANJVGYZ2M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>.
Download it today!
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Looks good @shao-xie . Thanks for putting this together. |
Summary
This proposal recommends splitting that the single Ossie ontology document into three distinct documents: an ontology document, a semantic model document (already standalone today), and a new mapping document, connected by external references instead of embedding. That lets one ontology map to many semantic models, and one semantic model be reused by many ontologies, without duplicating either, and brings the ontology specification in line with the community's recent alignment on one semantic model per document.
The core spec already states one semantic model per document, with no bundling and no cross-model references (core-spec/spec.md). PR #383 made that concrete for every semantic model document except the one still embedded inside an ontology's ontology_mappings, which it explicitly carved out and deferred.
This closes that gap. A mapping can now be written as its own document, validated against the new ontology/mapping.json, instead of embedding a full semantic model inside ontology_mappings. It references exactly one ontology and exactly one semantic model by {name, iri}, name checked against the referenced document's own name, iri saying where to resolve it from. An ontology mapped to more than one semantic model becomes one mapping document per semantic model, each independently owned, rather than one shared ontology_mappings list every owner has to edit.
This does not remove or change the existing embedded shape: ontology_mappings/OntologyMap keeps validating exactly as it does today, now annotated
deprecated: trueand pointing at the standalone alternative. Nokinddiscriminator is introduced; a mapping document is recognized the same way the existing two document kinds are, structurally, by havingconcept_mappings, a field neither an ontology nor a semantic model document has.examples/flights.yaml is split into its three parts (flights.ontology.yaml, flights.semantic_model.yaml, flights.mapping.yaml) as a worked example, byte-identical to the original content, only regrouped. All three, plus the existing tpcds/flights examples, validate cleanly with validation/validate.py against their respective schemas.
Related Issues
Close the gap of PR #383
Checklist
Specification
core-spec/and follow the existing structureOntology
ontology/are consistent with spec changesConverters
converters/is updated to reflect spec or ontology changesValidation
validation/are updated if the spec changedDocumentation
docs/is updated to reflect any user-facing changesCONTRIBUTING.mdis updated if the contribution process changedExamples
examples/are added or updated for any new spec constructs or converter supportTests
pytest/ CI green)Compliance