Repository navigation
Define one semantic model per document without a wrapper - #383
Conversation
flyrain
changed the title
|
The shape change is fine by me. I.e. a document-boundary change, not a model-semantics one. I have One case worth settling first, because it is where "one model per document" meets real vendor A chasm trap: Ossie → vendor is unaffected. ThoughtSpot resolves multi-fact chasm traps in its query engine, Vendor → Ossie is the problem. A modeller may legitimately have split those into two This PR does not create that, but it makes it permanent: one model per document and no bundle Which is intended?
Either is defensible. It decides whether a converter should silently duplicate the shared |
There was a problem hiding this comment.
🔵 Needs a closer look
The breaking schema change conflicts with existing consumers, while SDK and converter migration remains deferred.
Pull request overview
This PR changes standalone documents to contain one root-level semantic model, updating the schema, validator, documentation, examples, and tests.
Changes:
- Removes the standalone
semantic_modelwrapper. - Updates validation and regression coverage.
- Flattens specifications and the TPC-DS example while preserving embedded models.
A critical compatibility concern remains because existing SDK, converter, and consumer flows still expect the wrapped format.
File summaries
| File | Description |
|---|---|
validation/validate.py |
Traverses and validates root-level models. |
validation/tests/test_validate.py |
Tests new document shapes and traversal. |
validation/test_validate.py |
Adds CLI regression coverage. |
examples/tpcds_semantic_model.yaml |
Flattens the canonical example. |
docs/index.md |
Updates the overview and migration guidance. |
core-spec/spec.yaml |
Updates the YAML reference structure. |
core-spec/spec.md |
Documents the schema and migration guidance. |
core-spec/ossie-schema.json |
Defines the flat standalone document shape. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@flyrain Could you fix this please ? |
| # Semantic checks rely on valid structure; let schema validation report | ||
| # malformed inputs (including legacy arrays) without traversing them. | ||
| if not errors and isinstance(data, dict) and "datasets" in data: |
There was a problem hiding this comment.
Could you create an issue to fix this in a follow up ?
@djwaldo, thanks for raising this. The existing array groups models in one file, but it doesn’t define shared dataset identity across those models. Two copies of product aren’t currently declared to be the same dataset just because they appear in the same document. This PR changes how the models are packaged while leaving that limitation unchanged.
I see your two options as compatible: cross-model dataset identity is outside the current core spec, and we can add support for it separately. One model per doc still leaves room for shared identifiers, references, or a manifest in the future. If we introduce a shared identity would need an explicit design beyond simply grouping files. |
|
@khush-bhatia thanks a lot for the quick review. Resolved all comments. Pls take another look. |
| | `dialects` | array | No | Expression dialects used in the document | | ||
| | `vendors` | array | No | Vendors with custom extensions in the document | |
There was a problem hiding this comment.
vendors and dialects should be removed.
| Each JSON or YAML document represents exactly one semantic model. Model properties | ||
| are defined directly at the document root alongside `version`, `dialects`, and | ||
| `vendors`; there is no `semantic_model` wrapper. A model can contain multiple | ||
| datasets, relationships, and metrics. |
There was a problem hiding this comment.
Please remove dialects, and vendors from here.
| The reusable `$defs/SemanticModel` schema still describes model contents without | ||
| standalone document metadata. In particular, an ontology map continues to embed | ||
| those contents under its `semantic_model` property. This standalone document | ||
| change does not rename or flatten that ontology property. |
There was a problem hiding this comment.
Instead we should update the ontology spec to point to the root of this spec(perhaps "https://github.com/apache/ossie/core-spec/ossie-schema.json") ?
Can be done in a follow up PR though.
There was a problem hiding this comment.
Agreed, let’s handle that in a follow-up PR. Referencing the root schema would also require version on each embedded semantic model, so we should update the ontology examples, converters, and tests together. I’ll leave the current reference in place for this PR.
4826881 to
193c707
Compare
|
@khush-bhatia , fixed as suggested. PTAL. |
|
Thanks for clarification. |
|
Merging this branch (6470653) with current main (fc6c9df): #375 landed yesterday, after the last push here, and its Same fix this PR applies to the other three validators: drop the |
6470653 to
3fbf671
Compare
|
Thanks @kayemkim , fixed it in the new commit. I'd suggest to merge this to avoid further rebase. |
Follows apache#383, which moved the semantic model's fields to the document root and removed the `semantic_model` wrapper. `ossie-schema.json` now requires `version`, `name` and `datasets` at the root and forbids additional properties, so the converter's emitted documents stopped validating the moment the branch was rebased. Emit side is `{"version": ..., **semantic_model}`. The spread cannot clobber `version`: the model dict only ever holds name/description/datasets/ relationships/metrics plus the `custom_extensions` the stash writes, and no path on it emits a `version` key. Read side rejects the old wrapper by name rather than lifting its first entry, matching the treatment in apache#396. Reading entry zero would convert silently while discarding any later model, and a document old enough to carry the wrapper may have moved on elsewhere too; naming the one thing the reader must change is more useful than a best-effort guess. `datasets` is `minItems: 1` upstream, so the emit side now reports TS-MODEL-NO-DATASETS at ERROR when a model yields none. Without it the two legs disagreed: `to-ossie` exited 0 having written a document that fails the schema and that this converter's own `to-tml` refuses. Both legs now exit 1 on the same input. Both fixtures were regenerated by running the migrated converter over their own TML and asserting the result deep-equals the old expected document with its wrapper lifted -- so the 1,151 changed fixture lines are a re-rooting, not a content change. 823 tests pass. The README's mapping row is now pinned by a test, having been verified by nothing: reverting it left the whole suite green. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
apache#383 moved the Ossie document schema to a single semantic model defined directly at the document root, and apache#397 dropped root-level `dialects` and `vendors`. The Honeydew converter still required the `semantic_model: [...]` wrapper, so it could not read any current Ossie document — including the TPC-DS example, which its own round-trip test loads. Read the model from the document root, emit `version` plus the model's properties with no wrapper, and drop the `vendors` round-trip (the field no longer exists in the schema, and `additionalProperties: false` now rejects it). A document that still carries the legacy wrapper gets an error that says how to migrate it. The dropped multi-model warning has no meaning now that a document holds exactly one model. Part of #418 Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
PR #383 moved the Ossie document schema to one semantic model directly at the document root (dropping the `semantic_model:` wrapper), and #407 fixed the Microsoft converter for it, but the Sigma and NVIDIA GSF converters were never updated, so their CI failed on main as soon as an unrelated change touched their paths and re-triggered it. The Sigma, dbt, and wisdom converters share the `ossie` Python package's OssieDocument/OssieSemanticModel pydantic models, so fixing OssieDocument to match the flat schema required updating all three converters (and their tests/snapshots) together to keep them internally consistent. NVIDIA GSF parses Ossie YAML directly against a dict shape and is fixed independently. Co-Authored-By: Claude Sonnet 5 <[email protected]>
Follows apache#383, which moved the semantic model's fields to the document root and removed the `semantic_model` wrapper. `ossie-schema.json` now requires `version`, `name` and `datasets` at the root and forbids additional properties, so the converter's emitted documents stopped validating the moment the branch was rebased. Emit side is `{"version": ..., **semantic_model}`. The spread cannot clobber `version`: the model dict only ever holds name/description/datasets/ relationships/metrics plus the `custom_extensions` the stash writes, and no path on it emits a `version` key. Read side rejects the old wrapper by name rather than lifting its first entry, matching the treatment in apache#396. Reading entry zero would convert silently while discarding any later model, and a document old enough to carry the wrapper may have moved on elsewhere too; naming the one thing the reader must change is more useful than a best-effort guess. `datasets` is `minItems: 1` upstream, so the emit side now reports TS-MODEL-NO-DATASETS at ERROR when a model yields none. Without it the two legs disagreed: `to-ossie` exited 0 having written a document that fails the schema and that this converter's own `to-tml` refuses. Both legs now exit 1 on the same input. Both fixtures were regenerated by running the migrated converter over their own TML and asserting the result deep-equals the old expected document with its wrapper lifted -- so the 1,151 changed fixture lines are a re-rooting, not a content change. 823 tests pass. The README's mapping row is now pinned by a test, having been verified by nothing: reverting it left the whole suite green. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
apache#383 moved the one semantic model a document holds to its root and dropped the `semantic_model` wrapper. The converter still wrote the wrapper on import and required it on export, so every imported model failed the spec validator and every current document was refused on export -- which is what the generated round-trip tests tripped on in CI. Both directions now use the root mapping as the model. The "several models, only the first converted" issue goes with the wrapper, since a document can no longer hold more than one. Fixtures, inline test YAML and the hand-authored model generator are moved to the flat format.
…paths Follow main's move to flat single-model documents (apache#383, apache#396): - Import writes the model's properties at the document root beside `version` instead of inside a `semantic_model` list. - Export reads the model from the root. The legacy wrapped layout (Apache Ossie 0.1.1, early 0.2.0.dev0 snapshots) is still read, with a warning; an empty or non-list wrapper raises, and extra models warn. - Import's "did you mean `ossie-solid export`?" hint now recognizes a flat Ossie document as well as a wrapped one. - Fixtures flattened; tpcds_ossie.yaml regenerated, which also drops its duplicated `version:` line. The cross-vendor sweep detects flat documents again (10 fixtures across 5 converters) instead of skipping. Read OSSIE_SQL_2026 (apache#439) as a portable, ANSI-compatible dialect: - Expression fallback is the requested dialect, then ANSI_SQL, then OSSIE_SQL_2026; the fallback warning names the dialect actually read. - OSSIE_SQL_2026 is neither resolved to nor reported as unreadable, and maps to sqlglot's default tokenizer. Trigger the Solid CI on core-spec/**, examples/** and python/** changes, since the suite validates against the schema and the shared examples. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…#417) PR #383 moved the Ossie document schema to one semantic model directly at the document root (dropping the `semantic_model:` wrapper), and #407 fixed the Microsoft converter for it, but the Sigma and NVIDIA GSF converters were never updated, so their CI failed on main as soon as an unrelated change touched their paths and re-triggered it. The Sigma, dbt, and wisdom converters share the `ossie` Python package's OssieDocument/OssieSemanticModel pydantic models, so fixing OssieDocument to match the flat schema required updating all three converters (and their tests/snapshots) together to keep them internally consistent. NVIDIA GSF parses Ossie YAML directly against a dict shape and is fixed independently. Co-authored-by: Claude Sonnet 5 <[email protected]>
Follow main's move to flat single-model documents (apache#383, apache#396): - Import writes the model's properties at the document root beside `version` instead of inside a `semantic_model` list. - Export reads the model from the root. The legacy wrapped layout (Apache Ossie 0.1.1, early 0.2.0.dev0 snapshots) is still read, with a warning; an empty or non-list wrapper raises, and extra models warn. - Import's "did you mean `ossie-solid export`?" hint now recognizes a flat Ossie document as well as a wrapped one. - Fixtures flattened; tpcds_ossie.yaml regenerated, which also drops its duplicated `version:` line. The cross-vendor sweep detects flat documents again (10 fixtures across 5 converters) instead of skipping. Read OSSIE_SQL_2026 (apache#439) as a portable, ANSI-compatible dialect: - Expression fallback is the requested dialect, then ANSI_SQL, then OSSIE_SQL_2026; the fallback warning names the dialect actually read. - OSSIE_SQL_2026 is neither resolved to nor reported as unreadable, and maps to sqlglot's default tokenizer. Trigger the Solid CI on core-spec/**, examples/** and python/** changes, since the suite validates against the schema and the shared examples. Align the workflow with the normalized converter workflows (apache#480): read-only permissions, fail-fast off, and uv from the pinned setup-uv action. List converters/solid among the uv projects in dependabot.yml (apache#497). Co-Authored-By: Claude Opus 5.5 <[email protected]>
* fix(osi): export the current Apache Ossie document shape Apache Ossie replaced the semantic_model array with one model at the document root (apache/ossie#383) without changing 0.2.0.dev0, so every exported document failed the current schema and validate.py. - Export puts the model's properties at the root. - The vendored schema is the current upstream one: flat root, the HOLISTICS_AQL dialect (read as non-SQL), non-empty names, sources and keys. - The validator checks duplicate names and dataset references in flat documents, which it skipped; a wrapped document is still walked so a legacy input reports every problem. - Import still reads the semantic_model array, converting its first model, now with a warning. - Tests and fixtures move to the flat shape; the TPC-DS examples are the current upstream ones, and the TPC-DS OBML sample is regenerated. - Docs describe the flat shape and drop the claim that the export writes root dialects/vendors arrays. * fix(osi): export nested-only data objects, guard malformed identifiers A data object with nestedIn and no code exported source "", which the current schema rejects (minLength 1). Ossie allows a query as the source, so it now exports the unnest in the base dialect's form, selecting each column's code from the element; a nested parent becomes a derived table and a struct path is split per segment. An obml_source_unnest flag lets the import restore the object without code, database or schema. The query runs as written on Postgres (verified); other engines spell the unnest differently, which the docs state. The validator hashed dataset names and relationship ends, so a non-string identifier crashed it with TypeError before the schema errors came back. The semantic checks now consider only non-empty string identifiers; anything else is already a schema error. * fix(osi): nested source queries run on Postgres for chains and struct paths A nested parent's subquery projected only its declared columns, so a child unnesting from an undeclared field (Charge Labels -> x_Meta.Notes) referenced a column the subquery dropped. The subquery now also selects the first path segment of every object nested in it. A struct path rendered as "C"."Meta"."Labels", which Postgres reads as a three-part table name. It now uses composite field access, ("C"."Meta")."Labels", as OrionBelt's Postgres dialect does. Verified on Postgres with a three-level chain ending in a struct path and a struct path off the root table. * fix(osi): select each physical field once in a nested source query Two OBML columns may share a code (Label Key and Label Code over Key). The nested source projected it twice, so reading either exported field failed on Postgres with an ambiguous column reference. The projection now lists each code once; both fields still export.
Summary
Change the core JSON/YAML specification so each standalone document contains exactly one semantic model, with model properties directly at the root. Remove both the
semantic_modelarray and its wrapper. For example:A model still supports multiple datasets, relationships, and metrics. Bulk exchange uses separate documents; this PR does not introduce a bundle format.
Breaking change to the unreleased, mutable
0.2.0.dev0schema: wrapped models (arrays or objects) are rejected. Migration moves a single model's properties to the root or splits multiple models into separate documents, preserving model contents and using the current specification version. Root-leveldialectsandvendorsdeclarations remain removed, as established by #397.Changes
$defs/SemanticModeland the ontology map's embeddedsemantic_modelproperty.Scope and merge coordination: The Python SDK and converter migration is implemented in PR #396, which also updates converter CI to run on shared schema/example changes. Review the spec and migration separately, but do not land this schema change until #396 is approved and its combined spec/consumer CI is green. Coordinate the two merges so the repository is not left with the new schema and old consumers. This PR alone does not migrate those consumers.
Applicable checklist
core-spec/.