osi converter: preserve N dimensions over one column across the round-trip (#222)

packages/osi-orionbelt/src/osi_orionbelt/obml_to_osi.py

@@ -388,10 +388,18 @@ def _convert_column(
             ext_data["obml_comment"] = col_obj["comment"]
         if col_obj.get("owner"):
             ext_data["obml_owner"] = col_obj["owner"]
-        # Preserve OBML-only dimension properties (timeGrain, format, resultType, etc.)
+        # Preserve OBML-only dimension properties (timeGrain, format, resultType, etc.).
+        # OBML allows N dimensions over one column (grain variants, role-playing
+        # via ``via``); OSI is field-centric (one dimension per field). The first
+        # match is the primary dimension carried on the field; any extras are
+        # preserved as descriptors so the reverse trip can rebuild them instead of
+        # dropping them silently.
         matched_dim: dict[str, Any] | None = None
+        extra_dims: list[dict[str, Any]] = []
         for _dim_name, dim_obj in obml_dimensions.items():
-            if dim_obj.get("dataObject") == do_name and dim_obj.get("column") == col_name:
+            if dim_obj.get("dataObject") != do_name or dim_obj.get("column") != col_name:
+                continue
+            if matched_dim is None:
                 matched_dim = dim_obj
                 # Preserve the dimension's OBML name explicitly. The OSI field
                 # name is the physical code, so without this the round-trip
@@ -410,7 +418,38 @@ def _convert_column(
                     ext_data["obml_dimension_owner"] = dim_obj["owner"]
                 if dim_obj.get("via"):
                     ext_data["obml_dimension_via"] = dim_obj["via"]
-                break
+                # The dimension's own synonyms and vendor extensions have no
+                # native OSI slot (OSI has no dimension entity), so preserve them
+                # authoritatively here for the reverse trip. The customExtensions
+                # are also emitted as field foreign extensions below for OSI-tool
+                # visibility; that path lands them on the column on re-import,
+                # while this one restores them to the dimension.
+                if dim_obj.get("synonyms"):
+                    ext_data["obml_dimension_synonyms"] = dim_obj["synonyms"]
+                if dim_obj.get("customExtensions"):
+                    ext_data["obml_dimension_custom_extensions"] = dim_obj["customExtensions"]
+            else:
+                descriptor: dict[str, Any] = {"name": _dim_name}
+                for prop in ("resultType", "timeGrain", "format", "description", "owner", "via"):
+                    if dim_obj.get(prop):
+                        descriptor[prop] = dim_obj[prop]
+                # Carry the extra dimension's own synonyms and vendor extensions
+                # too, so it round-trips with the same fidelity as the primary.
+                if dim_obj.get("synonyms"):
+                    descriptor["synonyms"] = dim_obj["synonyms"]
+                if dim_obj.get("customExtensions"):
+                    descriptor["customExtensions"] = dim_obj["customExtensions"]
+                extra_dims.append(descriptor)
+        if extra_dims:
+            ext_data["obml_extra_dimensions"] = extra_dims
+            extra_names = ", ".join(d["name"] for d in extra_dims)
+            self.warnings.append(
+                f"Column '{do_name}.{col_name}' backs {len(extra_dims) + 1} OBML "
+                f"dimensions; OSI represents one dimension per field, so the "
+                f"{len(extra_dims)} additional dimension(s) ({extra_names}) are "
+                f"preserved via an OBSL extension for the reverse conversion but "
+                f"are not natively visible to other OSI tools."
+            )
         field["custom_extensions"] = [
             {
                 "vendor_name": _VENDOR_OBML,

packages/osi-orionbelt/src/osi_orionbelt/osi_to_obml.py

@@ -566,6 +566,7 @@ def _extract_dimensions(self, datasets: list) -> dict:
                     dim_def["synonyms"] = list(ai_ctx["synonyms"])
                 # Restore OBML-only dimension properties from custom_extensions
                 restored_name: str | None = None
+                extra_descriptors: list[Any] = []
                 for ext in field.get("custom_extensions", []):
                     if ext.get("vendor_name") in _OBML_VENDOR_READ:
                         try:
@@ -590,6 +591,24 @@ def _extract_dimensions(self, datasets: list) -> dict:
                                 dim_def["owner"] = ext_data["obml_dimension_owner"]
                             if ext_data.get("obml_dimension_via"):
                                 dim_def["via"] = ext_data["obml_dimension_via"]
+                            # The dimension's own synonyms / vendor extensions,
+                            # restored authoritatively to the dimension. Opaque
+                            # foreign data, so keep only well-shaped entries.
+                            _syns = ext_data.get("obml_dimension_synonyms")
+                            if isinstance(_syns, list):
+                                clean_syns = [s for s in _syns if isinstance(s, str) and s]
+                                if clean_syns:
+                                    dim_def["synonyms"] = clean_syns
+                            _exts = ext_data.get("obml_dimension_custom_extensions")
+                            if isinstance(_exts, list):
+                                clean_exts = [e for e in _exts if isinstance(e, dict)]
+                                if clean_exts:
+                                    dim_def["customExtensions"] = clean_exts
+                            # Additional dimensions over the same column, preserved
+                            # by the export because OSI has no slot for them.
+                            _extras = ext_data.get("obml_extra_dimensions")
+                            if isinstance(_extras, list):
+                                extra_descriptors = _extras
                         except (json.JSONDecodeError, TypeError):
                             pass
                         break
@@ -603,27 +622,67 @@ def _extract_dimensions(self, datasets: list) -> dict:
                     if not dim_def["synonyms"]:
                         del dim_def["synonyms"]
                 base_name = restored_name or field_name
-                # Dimension names must be unique across the model. When the same
-                # name occurs in more than one data object (e.g. foreign OSI where
-                # two datasets share a bare field name and no OBML-origin name was
-                # restored), qualify the later one with its data object instead of
-                # silently overwriting the earlier dimension. A restored OBML name
-                # is unique by construction, so this fallback is foreign-OSI only.
-                key = base_name
-                if key in dimensions and dimensions[key].get("dataObject") != ds_name:
-                    key = f"{ds_name} {base_name}"
-                    suffix = 2
-                    while key in dimensions:
-                        key = f"{ds_name} {base_name} {suffix}"
-                        suffix += 1
-                    self.warnings.append(
-                        f"Dimension name '{base_name}' occurs in multiple data "
-                        f"objects; emitted '{ds_name}.{base_name}' as dimension "
-                        f"'{key}' to avoid a collision."
-                    )
-                dimensions[key] = dim_def
+                self._insert_dimension(dimensions, ds_name, base_name, dim_def)
+                # Rebuild any additional OBML dimensions the export preserved for
+                # this column (OSI is one-dimension-per-field). Each descriptor is
+                # opaque foreign-modifiable data, so guard its shape.
+                for desc in extra_descriptors:
+                    if not isinstance(desc, dict):
+                        continue
+                    dname = desc.get("name")
+                    if not (isinstance(dname, str) and dname):
+                        continue
+                    extra_def: dict[str, Any] = {
+                        "dataObject": ds_name,
+                        "column": field_name,
+                        "resultType": desc.get("resultType") or abstract_type,
+                    }
+                    for prop in ("timeGrain", "format", "description", "owner", "via"):
+                        value = desc.get(prop)
+                        if isinstance(value, str) and value:
+                            extra_def[prop] = value
+                    # Restore the extra dimension's own synonyms / vendor
+                    # extensions. Opaque foreign data, so keep only well-shaped
+                    # entries (string synonyms; dict extensions).
+                    syns = desc.get("synonyms")
+                    if isinstance(syns, list):
+                        clean_syns = [s for s in syns if isinstance(s, str) and s]
+                        if clean_syns:
+                            extra_def["synonyms"] = clean_syns
+                    exts = desc.get("customExtensions")
+                    if isinstance(exts, list):
+                        clean_exts = [e for e in exts if isinstance(e, dict)]
+                        if clean_exts:
+                            extra_def["customExtensions"] = clean_exts
+                    self._insert_dimension(dimensions, ds_name, dname, extra_def)
         return dimensions
 
+    def _insert_dimension(
+        self, dimensions: dict[str, Any], ds_name: str, base_name: str, dim_def: dict[str, Any]
+    ) -> None:
+        """Insert ``dim_def`` under a unique key.
+
+        Dimension names must be unique across the model. When ``base_name``
+        already names a dimension on a *different* data object (foreign OSI where
+        two datasets share a bare field name and no OBML-origin name was
+        restored), qualify the later one with its data object and warn instead of
+        silently overwriting the earlier dimension. A restored OBML name is unique
+        by construction, so the qualification is foreign-OSI only.
+        """
+        key = base_name
+        if key in dimensions and dimensions[key].get("dataObject") != ds_name:
+            key = f"{ds_name} {base_name}"
+            suffix = 2
+            while key in dimensions:
+                key = f"{ds_name} {base_name} {suffix}"
+                suffix += 1
+            self.warnings.append(
+                f"Dimension name '{base_name}' occurs in multiple data "
+                f"objects; emitted '{ds_name}.{base_name}' as dimension "
+                f"'{key}' to avoid a collision."
+            )
+        dimensions[key] = dim_def
+
     def _convert_metrics(self, osi_metrics: list, ds_map: dict) -> tuple[dict, dict]:
         """
         Convert OSI metrics to OBML measures and metrics.

packages/osi-orionbelt/tests/test_osi_converter_roundtrip_robustness.py

@@ -333,6 +333,33 @@ def test_restored_name_not_left_as_a_synonym(self) -> None:
         for name, dim in obml["dimensions"].items():
             assert name not in dim.get("synonyms", [])
 
+    def test_primary_dimension_synonyms_and_extensions_preserved(self) -> None:
+        # The primary (single) dimension on a column keeps its own synonyms and
+        # vendor extensions across the round-trip, not just the scalar props.
+        obml = {
+            "version": 1.0,
+            "dataObjects": {
+                "Orders": {
+                    "code": "o",
+                    "database": "W",
+                    "schema": "P",
+                    "columns": {"Status": {"code": "status", "abstractType": "string"}},
+                }
+            },
+            "dimensions": {
+                "Status": {
+                    "dataObject": "Orders",
+                    "column": "Status",
+                    "synonyms": ["state", "condition"],
+                    "customExtensions": [{"vendor": "TABLEAU", "data": '{"role": "dim"}'}],
+                }
+            },
+        }
+        back = conv.OSItoOBML(conv.OBMLtoOSI(obml).convert()).convert()
+        dim = back["dimensions"]["Status"]
+        assert dim.get("synonyms") == ["state", "condition"]
+        assert dim.get("customExtensions") == [{"vendor": "TABLEAU", "data": '{"role": "dim"}'}]
+
     @pytest.mark.parametrize("bad", [["x"], 5, "", {}, None])
     def test_non_string_restored_name_is_ignored(self, bad: Any) -> None:
         # obml_dimension_name is opaque to validate_osi, so a foreign payload may
@@ -371,6 +398,165 @@ def test_non_string_restored_name_is_ignored(self, bad: Any) -> None:
         assert list(obml["dimensions"]) == ["dt"]
 
 
+# ---------------------------------------------------------------------------
+# #222: N OBML dimensions over one column
+# ---------------------------------------------------------------------------
+
+
+class TestMultipleDimensionsPerColumn:
+    """OBML allows N dimensions over one column (grain variants, role-playing);
+    OSI is one-dimension-per-field. The export preserves the extras via an
+    extension and warns (not silent); the import rebuilds them all (#222)."""
+
+    _OBML = {
+        "version": 1.0,
+        "dataObjects": {
+            "Orders": {
+                "code": "FACT",
+                "database": "WH",
+                "schema": "PUBLIC",
+                "columns": {
+                    "date": {"code": "order_date", "abstractType": "date"},
+                    "Amount": {"code": "amt", "abstractType": "float"},
+                },
+            }
+        },
+        "dimensions": {
+            "Order Day": {
+                "dataObject": "Orders",
+                "column": "date",
+                "resultType": "date",
+                "timeGrain": "day",
+            },
+            "Order Month": {
+                "dataObject": "Orders",
+                "column": "date",
+                "resultType": "date",
+                "timeGrain": "month",
+            },
+            "Order Quarter": {
+                "dataObject": "Orders",
+                "column": "date",
+                "resultType": "date",
+                "timeGrain": "quarter",
+            },
+        },
+        "measures": {
+            "Revenue": {
+                "columns": [{"dataObject": "Orders", "column": "Amount"}],
+                "aggregation": "sum",
+                "resultType": "float",
+            }
+        },
+    }
+
+    def _roundtrip(self) -> tuple[dict[str, Any], list[str]]:
+        exp = conv.OBMLtoOSI(self._OBML, model_name="s")
+        osi = exp.convert()
+        return conv.OSItoOBML(osi).convert(), exp.warnings
+
+    def test_all_dimensions_survive_with_distinct_grains(self) -> None:
+        obml, _ = self._roundtrip()
+        dims = obml["dimensions"]
+        assert set(dims) == {"Order Day", "Order Month", "Order Quarter"}
+        assert {d["timeGrain"] for d in dims.values()} == {"day", "month", "quarter"}
+        # All three still point at the same physical column.
+        assert {d["column"] for d in dims.values()} == {"order_date"}
+
+    def test_export_warns_not_silent(self) -> None:
+        _, warnings = self._roundtrip()
+        assert any("backs 3 OBML dimensions" in w for w in warnings), warnings
+
+    def test_extra_dimension_synonyms_and_extensions_preserved(self) -> None:
+        # An extra dimension's own synonyms and vendor extensions must survive,
+        # matching the primary's fidelity (not just the scalar props).
+        obml = {
+            "version": 1.0,
+            "dataObjects": {
+                "Orders": {
+                    "code": "F",
+                    "database": "W",
+                    "schema": "P",
+                    "columns": {"date": {"code": "dt", "abstractType": "date"}},
+                }
+            },
+            "dimensions": {
+                "Order Day": {
+                    "dataObject": "Orders",
+                    "column": "date",
+                    "resultType": "date",
+                    "timeGrain": "day",
+                },
+                "Order Month": {
+                    "dataObject": "Orders",
+                    "column": "date",
+                    "resultType": "date",
+                    "timeGrain": "month",
+                    "synonyms": ["monthly date"],
+                    "customExtensions": [{"vendor": "ACME", "data": '{"y": 2}'}],
+                },
+            },
+        }
+        back = conv.OSItoOBML(conv.OBMLtoOSI(obml, model_name="s").convert()).convert()
+        month = back["dimensions"]["Order Month"]
+        assert month.get("synonyms") == ["monthly date"]
+        assert month.get("customExtensions") == [{"vendor": "ACME", "data": '{"y": 2}'}]
+
+    @pytest.mark.parametrize(
+        "bad",
+        [
+            "not-a-list",
+            [5],
+            [{"no_name": 1}],
+            [{"name": ["x"]}],
+            [{"name": ""}],
+            [{"name": "OK"}, 7],
+            [{"name": "OK", "synonyms": "not-a-list"}],
+            [{"name": "OK", "synonyms": [1, {}]}],
+            [{"name": "OK", "customExtensions": "bad"}],
+            [{"name": "OK", "customExtensions": [7]}],
+        ],
+    )
+    def test_malformed_extra_dimensions_never_crash(self, bad: Any) -> None:
+        # obml_extra_dimensions is opaque to validate_osi. A malformed payload
+        # (not a list, or items that are not dicts / lack a string name) must be
+        # skipped, never crash the converter on an unhashable key.
+        osi = {
+            "version": "0.2.0.dev0",
+            "semantic_model": [
+                {
+                    "name": "s",
+                    "datasets": [
+                        {
+                            "name": "Orders",
+                            "source": "WH.PUBLIC.orders",
+                            "fields": [
+                                {
+                                    "name": "dt",
+                                    "expression": {
+                                        "dialects": [{"dialect": "ANSI_SQL", "expression": "dt"}]
+                                    },
+                                    "dimension": {},
+                                    "custom_extensions": [
+                                        {
+                                            "vendor_name": "ORIONBELT",
+                                            "data": json.dumps({"obml_extra_dimensions": bad}),
+                                        }
+                                    ],
+                                }
+                            ],
+                        }
+                    ],
+                }
+            ],
+        }
+        obml = conv.OSItoOBML(osi).convert()
+        # The primary field-name dimension is always present; any well-formed
+        # extra (the {"name": "OK"} case) is rebuilt, malformed items skipped.
+        assert "dt" in obml["dimensions"]
+        assert all(isinstance(k, str) for k in obml["dimensions"])
+
+
 # ---------------------------------------------------------------------------
 # P2: validate_osi robustness on malformed input
 # ---------------------------------------------------------------------------