From 334105e801a5bb0b35346f66ede9409a2d8bf5ec Mon Sep 17 00:00:00 2001 From: Baruch Oxman Date: Fri, 18 Sep 2026 15:32:55 +0300 Subject: [PATCH] fix(honeydew): read and write flat semantic model documents MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #383 moved the Ossie document schema to a single semantic model defined directly at the document root, and #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) --- converters/honeydew/README.md | 4 +- .../honeydew/src/ossie_honeydew/converter.py | 39 +++------ .../tests/test_ossie_honeydew_converter.py | 86 ++++++------------- 3 files changed, 41 insertions(+), 88 deletions(-) diff --git a/converters/honeydew/README.md b/converters/honeydew/README.md index 4419862b..0b2c2073 100644 --- a/converters/honeydew/README.md +++ b/converters/honeydew/README.md @@ -36,7 +36,7 @@ Honeydew documents this integration from its own side under | Ossie concept | Honeydew concept | |-------------|-----------------| -| `semantic_model.name` | `workspace.yml name` | +| `name` (document root) | `workspace.yml name` | | `dataset` | Entity + dataset files under `schema//` | | `dataset.source` | `dataset.sql` | | `dataset.primary_key` | `entity.keys` | @@ -50,7 +50,7 @@ Honeydew documents this integration from its own side under | Honeydew concept | Ossie concept | |-----------------|-------------| -| `workspace.name` | `semantic_model.name` | +| `workspace.name` | `name` (document root) | | Entity + primary dataset | `dataset` | | `entity.keys` | `dataset.primary_key` (and `dataset.unique_keys`) | | `dataset.attributes` (columns) | `fields` with `ANSI_SQL` expression = column name | diff --git a/converters/honeydew/src/ossie_honeydew/converter.py b/converters/honeydew/src/ossie_honeydew/converter.py index 33793e74..45f0f707 100644 --- a/converters/honeydew/src/ossie_honeydew/converter.py +++ b/converters/honeydew/src/ossie_honeydew/converter.py @@ -43,7 +43,7 @@ _OSSIE_METADATA_SECTION = "ossie" # Workspaces written before the Ossie rebrand named the section "osi". Still # read it, so exporting such a workspace does not silently drop the fields it -# preserves (ai_context, label, unique_keys, custom_extensions, vendors). +# preserves (ai_context, label, unique_keys, custom_extensions). _LEGACY_OSSIE_METADATA_SECTION = "osi" _HD_ATTR_KEYS = ("display_name", "hidden", "folder", "format_string", "timegrain") @@ -98,24 +98,20 @@ def convert_ossie_to_honeydew(ossie_yaml_str: str) -> dict[str, str]: f"Unsupported Ossie version '{version_str}'. Supported: {SUPPORTED_OSSIE_VERSION}" ) - semantic_models = root.get("semantic_model") - if not isinstance(semantic_models, list) or not semantic_models: - raise HoneydewConversionError("'semantic_model' must be a non-empty list") - - if len(semantic_models) > 1: - warnings.warn( - f"Ossie YAML contains {len(semantic_models)} semantic models; " - "only the first will be converted" + if "semantic_model" in root: + raise HoneydewConversionError( + "Document uses the legacy 'semantic_model:' wrapper. An Ossie document now " + "defines exactly one semantic model at its root; move the model's properties " + "up to the document root alongside 'version'." ) - vendors = [v for v in (root.get("vendors") or []) if v != HONEYDEW_VENDOR] - return _model_to_files(semantic_models[0], extra_vendors=vendors) + return _model_to_files(root) -def _model_to_files(sm: dict[str, Any], *, extra_vendors: list[str] | None = None) -> dict[str, str]: +def _model_to_files(sm: dict[str, Any]) -> dict[str, str]: name = sm.get("name") if not name: - raise HoneydewConversionError("Missing 'name' in semantic model") + raise HoneydewConversionError("Missing 'name' in Ossie document") files: dict[str, str] = {} @@ -123,13 +119,12 @@ def _model_to_files(sm: dict[str, Any], *, extra_vendors: list[str] | None = Non if sm.get("description"): workspace["description"] = sm["description"] - # Preserve model-level ai_context, non-HONEYDEW custom_extensions, and extra vendors + # Preserve model-level ai_context and non-HONEYDEW custom_extensions model_ai_ctx = sm.get("ai_context") model_ext = [e for e in (sm.get("custom_extensions") or []) if e.get("vendor_name") != HONEYDEW_VENDOR] ws_meta = _build_ossie_metadata( ai_context=model_ai_ctx, custom_extensions=model_ext or None, - extra_vendors=extra_vendors or None, ) if ws_meta: workspace["metadata"] = [ws_meta] @@ -606,14 +601,7 @@ def convert_honeydew_to_ossie(workspace_dir: str) -> str: if ossie_metrics: sm["metrics"] = ossie_metrics - extra_vendors = ws_ossie_meta.get("vendors") or [] - vendors = [HONEYDEW_VENDOR] + [v for v in extra_vendors if v != HONEYDEW_VENDOR] - root: dict[str, Any] = { - "version": SUPPORTED_OSSIE_VERSION, - "vendors": vendors, - "semantic_model": [sm], - } - return _dump(root) + return _dump({"version": SUPPORTED_OSSIE_VERSION, **sm}) def _read_entity_dir(entity_dir: str, entity_name: str) -> dict[str, Any]: @@ -934,7 +922,6 @@ def _build_ossie_metadata( label: str | None = None, unique_keys: Any = None, custom_extensions: list | None = None, - extra_vendors: list[str] | None = None, ) -> dict[str, Any] | None: """Build a Honeydew metadata entry that stores Ossie-only fields for round-tripping.""" items: list[dict[str, Any]] = [] @@ -948,8 +935,6 @@ def _build_ossie_metadata( items.append({"name": "unique_keys", "value": json.dumps(unique_keys)}) if custom_extensions: items.append({"name": "custom_extensions", "value": json.dumps(custom_extensions)}) - if extra_vendors: - items.append({"name": "vendors", "value": json.dumps(extra_vendors)}) if not items: return None @@ -975,7 +960,7 @@ def _read_ossie_metadata(obj: dict[str, Any]) -> dict[str, Any]: result[key] = raw elif key == "label": result[key] = raw - elif key in ("unique_keys", "custom_extensions", "vendors"): + elif key in ("unique_keys", "custom_extensions"): try: result[key] = json.loads(raw) except (json.JSONDecodeError, TypeError): diff --git a/converters/honeydew/tests/test_ossie_honeydew_converter.py b/converters/honeydew/tests/test_ossie_honeydew_converter.py index d83523b4..064368af 100644 --- a/converters/honeydew/tests/test_ossie_honeydew_converter.py +++ b/converters/honeydew/tests/test_ossie_honeydew_converter.py @@ -52,7 +52,7 @@ def _ossie(model_dict): return yaml.dump( - {"version": OSSIE_VERSION, "semantic_model": [model_dict]}, + {"version": OSSIE_VERSION, **model_dict}, default_flow_style=False, sort_keys=False, ) @@ -134,13 +134,15 @@ def _write_workspace(tmp_dir, workspace_name, entities): def _ossie_roundtrip(model_dict, tmp_path): - """Ossie → Honeydew → Ossie; returns the semantic model dict.""" + """Ossie → Honeydew → Ossie; returns the round-tripped model without 'version'.""" files = convert_ossie_to_honeydew(_ossie(model_dict)) for rel_path, content in files.items(): p = tmp_path / rel_path p.parent.mkdir(parents=True, exist_ok=True) p.write_text(content) - return yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path)))["semantic_model"][0] + doc = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) + assert doc.pop("version") == OSSIE_VERSION + return doc def _honeydew_roundtrip(entities, tmp_path): @@ -596,24 +598,19 @@ def test_ossie_to_honeydew_metric_entity_hint_overrides_expression(): def test_ossie_to_honeydew_invalid_version_raises(): with pytest.raises(HoneydewConversionError, match="Unsupported"): - convert_ossie_to_honeydew("version: '9.9.9'\nsemantic_model:\n - name: m\n") + convert_ossie_to_honeydew("version: '9.9.9'\nname: m\ndatasets: []\n") -def test_ossie_to_honeydew_missing_semantic_model_raises(): - with pytest.raises(HoneydewConversionError): - convert_ossie_to_honeydew(f"version: '{OSSIE_VERSION}'\n") +def test_ossie_to_honeydew_missing_name_raises(): + with pytest.raises(HoneydewConversionError, match="Missing 'name'"): + convert_ossie_to_honeydew(f"version: '{OSSIE_VERSION}'\ndatasets: []\n") -def test_ossie_to_honeydew_multiple_models_warns(): - doc = yaml.dump({"version": OSSIE_VERSION, "semantic_model": [ - {"name": "m1", "datasets": []}, - {"name": "m2", "datasets": []}, - ]}) - with warnings.catch_warnings(record=True) as w: - warnings.simplefilter("always") - files = convert_ossie_to_honeydew(doc) - assert any("only the first" in str(x.message) for x in w) - assert yaml.safe_load(files["workspace.yml"]) == {"type": "workspace", "name": "m1"} +def test_ossie_to_honeydew_legacy_wrapper_raises(): + """The pre-#383 'semantic_model:' wrapper is no longer a valid document.""" + doc = yaml.dump({"version": OSSIE_VERSION, "semantic_model": [{"name": "m", "datasets": []}]}) + with pytest.raises(HoneydewConversionError, match="legacy 'semantic_model:' wrapper"): + convert_ossie_to_honeydew(doc) # ───────────────────────────────────────────────────────────────────────────── @@ -621,7 +618,7 @@ def test_ossie_to_honeydew_multiple_models_warns(): # ───────────────────────────────────────────────────────────────────────────── def _hd_root(sm): - return {"version": OSSIE_VERSION, "vendors": ["HONEYDEW"], "semantic_model": [sm]} + return {"version": OSSIE_VERSION, **sm} def _ansi(expr): @@ -813,8 +810,7 @@ def test_honeydew_to_ossie_missing_workspace_raises(tmp_path): def test_honeydew_to_ossie_missing_schema_dir_empty_model(tmp_path): (tmp_path / "workspace.yml").write_text(yaml.dump({"type": "workspace", "name": "ws"})) result = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) - assert result == {"version": OSSIE_VERSION, "vendors": ["HONEYDEW"], - "semantic_model": [{"name": "ws", "datasets": []}]} + assert result == {"version": OSSIE_VERSION, "name": "ws", "datasets": []} def test_honeydew_to_ossie_empty_metric_sql_skipped(tmp_path): @@ -824,7 +820,7 @@ def test_honeydew_to_ossie_empty_metric_sql_skipped(tmp_path): "datatype": "number", "sql": ""}]}]) with warnings.catch_warnings(record=True): result = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) - assert "metrics" not in result["semantic_model"][0] + assert "metrics" not in result def test_honeydew_to_ossie_duplicate_relations_deduplicated(tmp_path): @@ -839,7 +835,7 @@ def test_honeydew_to_ossie_duplicate_relations_deduplicated(tmp_path): "dataset_attrs": []}, ]) result = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) - assert len(result["semantic_model"][0].get("relationships", [])) == 1 + assert len(result.get("relationships", [])) == 1 def test_honeydew_to_ossie_relation_target_columns_are_unique_keys(tmp_path): @@ -855,7 +851,7 @@ def test_honeydew_to_ossie_relation_target_columns_are_unique_keys(tmp_path): {"name": "customers", "keys": ["id"], "key_dataset": "customers", "sql": "db.s.customers", "dataset_attrs": []}, ]) - sm = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path)))["semantic_model"][0] + sm = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) datasets = {ds["name"]: ds for ds in sm["datasets"]} rel = sm["relationships"][0] target_ds = datasets[rel["to"]] @@ -1002,8 +998,7 @@ def test_ossie_roundtrip_tpcds_example(tmp_path): p = tmp_path / rel_path p.parent.mkdir(parents=True, exist_ok=True) p.write_text(content) - result = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) - sm = result["semantic_model"][0] + sm = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) assert sm["name"] == "tpcds_retail_model" ds_names = {ds["name"] for ds in sm["datasets"]} assert "store_sales" in ds_names and "customer" in ds_names @@ -1417,31 +1412,6 @@ def test_connectionless_relation_warns(): } -# ───────────────────────────────────────────────────────────────────────────── -# Vendors round-trip -# ───────────────────────────────────────────────────────────────────────────── - -@pytest.mark.parametrize("input_vendors,expected_vendors", [ - (["SNOWFLAKE", "HONEYDEW"], ["HONEYDEW", "SNOWFLAKE"]), - (["SNOWFLAKE"], ["HONEYDEW", "SNOWFLAKE"]), - (["HONEYDEW"], ["HONEYDEW"]), -]) -def test_vendors_roundtrip(tmp_path, input_vendors, expected_vendors): - doc = yaml.dump({ - "version": OSSIE_VERSION, - "vendors": input_vendors, - "semantic_model": [{"name": "m", "datasets": []}], - }) - files = convert_ossie_to_honeydew(doc) - for rel_path, content in files.items(): - p = tmp_path / rel_path - p.parent.mkdir(parents=True, exist_ok=True) - p.write_text(content) - result = yaml.safe_load(convert_honeydew_to_ossie(str(tmp_path))) - assert result == {"version": OSSIE_VERSION, "vendors": expected_vendors, - "semantic_model": [{"name": "m", "datasets": []}]} - - # ───────────────────────────────────────────────────────────────────────────── # main() CLI smoke tests # ───────────────────────────────────────────────────────────────────────────── @@ -1451,9 +1421,8 @@ def test_main_ossie_to_honeydew(tmp_path): input_file = tmp_path / "model.yaml" input_file.write_text(yaml.dump({ "version": OSSIE_VERSION, - "semantic_model": [{"name": "m", "datasets": [ - {"name": "orders", "source": "db.s.orders", "fields": []} - ]}], + "name": "m", + "datasets": [{"name": "orders", "source": "db.s.orders", "fields": []}], })) output_dir = tmp_path / "out" result = subprocess.run( @@ -1482,11 +1451,11 @@ def test_main_honeydew_to_ossie(tmp_path): assert result.returncode == 0 assert yaml.safe_load(output_file.read_text()) == { "version": OSSIE_VERSION, - "vendors": ["HONEYDEW"], - "semantic_model": [{"name": "ws", "datasets": [ + "name": "ws", + "datasets": [ {"name": "orders", "source": "DB.S.ORDERS", "primary_key": ["id"], "unique_keys": [["id"]]}, - ]}], + ], } @@ -1494,9 +1463,8 @@ def test_main_path_traversal_rejected(tmp_path): import subprocess input_file = tmp_path / "model.yaml" input_file.write_text( - f"version: '{OSSIE_VERSION}'\nsemantic_model:\n" - " - name: m\n datasets:\n" - " - name: '../../evil'\n source: db.s.evil\n fields: []\n" + f"version: '{OSSIE_VERSION}'\nname: m\ndatasets:\n" + " - name: '../../evil'\n source: db.s.evil\n fields: []\n" ) output_dir = tmp_path / "out" result = subprocess.run(