Skip to content

Fix Sigma and NVIDIA GSF converters for flat semantic model documents - #417

Open
jbonofre wants to merge 1 commit into
mainfrom
fix/converters-flat-semantic-model
Open

jbonofre wants to merge 1 commit into
mainfrom
fix/converters-flat-semantic-model

Conversation

@jbonofre

Copy link
Copy Markdown
Member

Summary

main currently has two red CI checks: Converters Sigma CI and Converters NVIDIA GSF CI. Both broke because #383 moved the Ossie document schema to a single semantic model defined directly at the document root (dropping the semantic_model: list wrapper), and #407 followed up to fix the Microsoft converter for that shape — but the Sigma and NVIDIA GSF converters were never updated. Their CI simply hadn't re-run since #383 landed until an unrelated change (#416) touched their paths and re-triggered it, surfacing the break.

  • NVIDIA GSF: emitted/expected the old semantic_model: [...] wrapper; the official validator now rejects that shape ('name' is a required property, 'semantic_model' was unexpected).
  • Sigma: OssieDocument.model_validate(...) failed loading the (already-flattened) examples/tpcds_semantic_model.yaml fixture with semantic_model: Field required.

What changed

  • python/src/ossie/models.py: OssieDocument now extends OssieSemanticModel directly (version + the model's fields at the document root) instead of wrapping a semantic_model: list[OssieSemanticModel], matching the current core-spec/ossie-schema.json. It also drops the removed root-level dialects/vendors fields.
  • Sigma & NVIDIA GSF converters: updated to read/write the flat document shape; removed the now-impossible "multiple models per document" handling (EXTRA_MODEL_DROPPED, empty-model errors) since a document is always exactly one model now.
  • dbt & wisdom converters: also updated. Both share the ossie Python package's OssieDocument/OssieSemanticModel models, so changing OssieDocument's shape would have broken them the moment their (currently green, but stale) CI next ran. Fixed in the same PR to keep the shared package's consumers consistent, including regenerating their syrupy snapshots.
  • Removed the dialects=[...] argument dbt passed into OssieDocument (that field no longer exists at the document root) and the test assertions that exercised it.

All four converters plus the shared python/ package pass their full test suites locally with the exact commands CI uses (uv sync && uv run pytest): Sigma 90/90, NVIDIA GSF 77/77, dbt 106/106, wisdom 27/27, python 9/9.

Follow-up

While fixing this I found the same latent break in several other converters that haven't had CI re-triggered since #383: databricks, orionbelt, ontology (+ the root examples/flights.yaml), gooddata, omni, honeydew, and the Java-based salesforce/polaris. None of those are touched by this PR — filing a separate issue to track them.

Test plan

  • cd python && uv run --with pytest --with PyYAML pytest tests/test_models.py — 9 passed
  • cd converters/sigma && uv run pytest — 90 passed
  • cd converters/nvidia && uv run pytest — 77 passed
  • cd converters/dbt && uv run pytest — 106 passed
  • cd converters/wisdom && uv run pytest — 27 passed

🤖 Generated with Claude Code

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 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant