Python(feat): accept list values for multi-value metadata writes

python/lib/sift_client/_internal/low_level_wrappers/runs.py

@@ -186,7 +186,7 @@ async def create_adhoc_run(
         start_time: datetime | None = None,
         stop_time: datetime | None = None,
         tag_names: list[str] | None = None,
-        metadata: dict[str, str | float | bool] | None = None,
+        metadata: dict[str, str | float | bool | list[str]] | None = None,
         client_key: str | None = None,
     ) -> Run:
         """Create an adhoc run.

python/lib/sift_client/_tests/resources/test_runs.py

@@ -8,6 +8,7 @@
 """
 
 from datetime import datetime, timedelta, timezone
+from typing import NoReturn
 
 import pytest
 from google.protobuf.field_mask_pb2 import FieldMask
@@ -401,16 +402,24 @@ async def test_update_with_run_id_string(self, runs_api_async, new_run):
                 await runs_api_async.archive(new_run.id_)
 
     class TestMultiValueMetadata:
-        """Read-side tests for multi-value metadata.
-
-        The public write path is scalar-only (list values land in ENG-13281
-        Phase 3), so these tests seed duplicate-key metadata through the raw
-        `UpdateRun` proto. Storing multiple values per key requires the
-        backend's org-scoped `multi-value-metadata` flag; against a backend
-        without it the seed either fails or collapses to one value, and the
-        test skips instead of failing.
+        """Multi-value metadata tests: raw-proto seeding, public list writes,
+        and read-side getall.
+
+        Storing multiple values per key requires the backend's org-scoped
+        `multi-value-metadata` flag; against a backend without it the seed
+        either fails or collapses to one value, and the tests skip instead
+        of failing.
         """
 
+        @staticmethod
+        def _skip_if_flag_disabled(exc: AioRpcError) -> NoReturn:
+            if exc.code() == StatusCode.INVALID_ARGUMENT:
+                pytest.skip(
+                    "backend rejected duplicate metadata keys; "
+                    "multi-value-metadata flag not enabled"
+                )
+            raise exc
+
         @pytest.mark.asyncio
         async def test_get_run_exposes_all_values_via_getall(
             self, sift_client, runs_api_async, new_run
@@ -436,12 +445,7 @@ async def test_get_run_exposes_all_values_via_getall(
                         request
                     )
                 except AioRpcError as exc:
-                    if exc.code() == StatusCode.INVALID_ARGUMENT:
-                        pytest.skip(
-                            "backend rejected duplicate metadata keys; "
-                            "multi-value-metadata flag not enabled"
-                        )
-                    raise
+                    self._skip_if_flag_disabled(exc)
                 stored = [
                     entry.string_value
                     for entry in response.run.metadata
@@ -463,6 +467,47 @@ async def test_get_run_exposes_all_values_via_getall(
             finally:
                 await runs_api_async.archive(new_run.id_)
 
+        @pytest.mark.asyncio
+        async def test_update_run_with_list_metadata(self, runs_api_async, new_run):
+            """A list[str] value in RunUpdate writes every element under the key."""
+            values = ["flux_capacitor", "lightsaber"]
+            try:
+                update = RunUpdate(metadata={"part_number": values, "env": "prod"})
+                try:
+                    updated_run = await runs_api_async.update(new_run, update)
+                except AioRpcError as exc:
+                    self._skip_if_flag_disabled(exc)
+
+                assert updated_run.metadata["part_number"] == values[0]
+                assert updated_run.metadata.getall("part_number") == values
+                assert updated_run.metadata["env"] == "prod"
+
+                fetched = await runs_api_async.get(run_id=new_run.id_)
+                assert fetched.metadata.getall("part_number") == values
+            finally:
+                await runs_api_async.archive(new_run.id_)
+
+        @pytest.mark.asyncio
+        async def test_metadata_round_trip_preserves_all_values(self, runs_api_async, new_run):
+            """update(metadata=run.metadata) must not drop multi-value entries."""
+            values = ["flux_capacitor", "lightsaber"]
+            try:
+                try:
+                    await runs_api_async.update(
+                        new_run, RunUpdate(metadata={"part_number": values})
+                    )
+                except AioRpcError as exc:
+                    self._skip_if_flag_disabled(exc)
+
+                fetched = await runs_api_async.get(run_id=new_run.id_)
+                # Write the fetched Metadata mapping back unchanged.
+                round_tripped = await runs_api_async.update(
+                    new_run, RunUpdate(metadata=fetched.metadata)
+                )
+                assert round_tripped.metadata.getall("part_number") == values
+            finally:
+                await runs_api_async.archive(new_run.id_)
+
     class TestArchive:
         """Tests for the async archive method."""
 

python/lib/sift_client/_tests/sift_types/test_asset.py

@@ -7,6 +7,7 @@
 
 from sift_client.sift_types import Asset
 from sift_client.sift_types.asset import AssetUpdate
+from sift_client.util.metadata import Metadata
 
 
 class TestAssetUpdate:
@@ -31,6 +32,33 @@ def test_metadata_converter(self):
         assert metadata_dict["key3"].boolean_value is True
         assert "metadata" in mask.paths
 
+    def test_metadata_list_value_writes_every_element(self):
+        """A list[str] value produces one MetadataValue per element, in order."""
+        update = AssetUpdate(metadata={"parts": ["ABC", "XYZ"], "env": "prod"})
+        update.resource_id = "test_asset_id"
+
+        proto, mask = update.to_proto_with_mask()
+
+        assert [md.string_value for md in proto.metadata if md.key.name == "parts"] == [
+            "ABC",
+            "XYZ",
+        ]
+        assert [md.string_value for md in proto.metadata if md.key.name == "env"] == ["prod"]
+        assert "metadata" in mask.paths
+
+    def test_metadata_mapping_round_trips_all_values(self):
+        """Passing an entity's Metadata mapping must not drop multi-value entries."""
+        metadata = Metadata({"parts": "ABC"}, {"parts": ["ABC", "XYZ"]})
+        update = AssetUpdate(metadata=metadata)
+        update.resource_id = "test_asset_id"
+
+        proto, _ = update.to_proto_with_mask()
+
+        assert [md.string_value for md in proto.metadata if md.key.name == "parts"] == [
+            "ABC",
+            "XYZ",
+        ]
+
 
 @pytest.fixture
 def mock_asset(mock_client):

python/lib/sift_client/_tests/sift_types/test_run.py

@@ -7,6 +7,7 @@
 
 from sift_client.sift_types import Run
 from sift_client.sift_types.run import RunCreate, RunUpdate
+from sift_client.util.metadata import Metadata
 
 
 class TestRunCreate:
@@ -26,6 +27,24 @@ def test_metadata_converter(self):
         assert metadata_dict["number_key"].number_value == 3.14
         assert metadata_dict["bool_key"].boolean_value is True
 
+    def test_metadata_list_value_writes_every_element(self):
+        """A list[str] value produces one MetadataValue per element, in order."""
+        create = RunCreate(name="test_run", metadata={"parts": ["ABC", "XYZ"], "env": "prod"})
+        proto = create.to_proto()
+
+        assert [md.string_value for md in proto.metadata if md.key.name == "parts"] == [
+            "ABC",
+            "XYZ",
+        ]
+        assert [md.string_value for md in proto.metadata if md.key.name == "env"] == ["prod"]
+
+    def test_rust_form_rejects_list_metadata(self):
+        """The streaming ingestion path does not support multi-value metadata yet."""
+        pytest.importorskip("sift_stream_bindings")
+        create = RunCreate(name="test_run", metadata={"parts": ["ABC", "XYZ"]})
+        with pytest.raises(ValueError, match="streaming ingestion path"):
+            create._to_rust_form()
+
     def test_time_validator_start_before_stop(self):
         """Test time validator accepts start_time before stop_time."""
         start_time = datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
@@ -74,6 +93,34 @@ def test_metadata_converter(self):
         assert metadata_dict["key3"].boolean_value is False
         assert "metadata" in mask.paths
 
+    def test_metadata_list_value_writes_every_element(self):
+        """A list[str] value produces one MetadataValue per element, in order."""
+        update = RunUpdate(metadata={"parts": ["ABC", "XYZ"], "env": "prod"})
+        update.resource_id = "test_run_id"
+
+        proto, mask = update.to_proto_with_mask()
+
+        assert [md.string_value for md in proto.metadata if md.key.name == "parts"] == [
+            "ABC",
+            "XYZ",
+        ]
+        assert [md.string_value for md in proto.metadata if md.key.name == "env"] == ["prod"]
+        assert "metadata" in mask.paths
+
+    def test_metadata_mapping_round_trips_all_values(self):
+        """Passing an entity's Metadata mapping must not drop multi-value entries."""
+        metadata = Metadata({"parts": "ABC", "env": "prod"}, {"parts": ["ABC", "XYZ"]})
+        update = RunUpdate(metadata=metadata)
+        update.resource_id = "test_run_id"
+
+        proto, _ = update.to_proto_with_mask()
+
+        assert [md.string_value for md in proto.metadata if md.key.name == "parts"] == [
+            "ABC",
+            "XYZ",
+        ]
+        assert [md.string_value for md in proto.metadata if md.key.name == "env"] == ["prod"]
+
     def test_time_validator_start_before_stop(self):
         """Test time validator accepts start_time before stop_time."""
         start_time = datetime(2024, 1, 1, 12, 0, 0, tzinfo=timezone.utc)

python/lib/sift_client/_tests/util/test_metadata.py

@@ -8,7 +8,12 @@
     MetadataValue,
 )
 
-from sift_client.util.metadata import Metadata, metadata_dict_to_proto, metadata_proto_to_dict
+from sift_client.util.metadata import (
+    Metadata,
+    expand_metadata_for_write,
+    metadata_dict_to_proto,
+    metadata_proto_to_dict,
+)
 
 
 def _string_value(name: str, value: str) -> MetadataValue:
@@ -70,6 +75,60 @@ def test_round_trip_from_dict(self):
         assert metadata_proto_to_dict(metadata_dict_to_proto(original)) == original
 
 
+class TestMetadataDictToProto:
+    def test_list_value_writes_one_proto_per_element_in_order(self):
+        protos = metadata_dict_to_proto({"parts": ["ABC", "XYZ"], "env": "prod"})
+        parts = [p.string_value for p in protos if p.key.name == "parts"]
+        assert parts == ["ABC", "XYZ"]
+        assert all(
+            p.key.type == MetadataKeyType.METADATA_KEY_TYPE_STRING
+            for p in protos
+            if p.key.name == "parts"
+        )
+        assert [p.string_value for p in protos if p.key.name == "env"] == ["prod"]
+
+    def test_list_round_trips_through_proto(self):
+        protos = metadata_dict_to_proto({"parts": ["ABC", "XYZ"], "build": 1.5})
+        result = metadata_proto_to_dict(protos)
+        assert result == {"parts": "ABC", "build": 1.5}
+        assert result.getall("parts") == ["ABC", "XYZ"]
+
+    def test_metadata_instance_keeps_all_values(self):
+        md = Metadata({"parts": "ABC", "env": "prod"}, {"parts": ["ABC", "XYZ"]})
+        protos = metadata_dict_to_proto(md)
+        assert [p.string_value for p in protos if p.key.name == "parts"] == ["ABC", "XYZ"]
+        assert [p.string_value for p in protos if p.key.name == "env"] == ["prod"]
+
+    def test_empty_list_rejected(self):
+        with pytest.raises(ValueError, match="empty value list"):
+            metadata_dict_to_proto({"parts": []})
+
+    def test_non_string_list_element_rejected(self):
+        with pytest.raises(ValueError, match="only string values may be multi-value"):
+            metadata_dict_to_proto({"parts": ["ABC", 2]})  # type: ignore[list-item]
+
+    def test_unsupported_scalar_rejected(self):
+        with pytest.raises(ValueError, match="Unsupported metadata value type"):
+            metadata_dict_to_proto({"parts": None})  # type: ignore[dict-item]
+
+
+class TestExpandMetadataForWrite:
+    def test_multi_value_keys_become_lists_single_stay_scalar(self):
+        md = Metadata(
+            {"parts": "ABC", "env": "prod", "build": 1.5},
+            {"parts": ["ABC", "XYZ"], "env": ["prod"]},
+        )
+        assert expand_metadata_for_write(md) == {
+            "parts": ["ABC", "XYZ"],
+            "env": "prod",
+            "build": 1.5,
+        }
+
+    def test_plain_dict_returned_unchanged(self):
+        plain = {"parts": ["ABC", "XYZ"], "env": "prod"}
+        assert expand_metadata_for_write(plain) is plain
+
+
 class TestMetadataMapping:
     def test_dict_view_and_getall_invariant(self):
         md = Metadata(

python/lib/sift_client/sift_types/asset.py

@@ -3,12 +3,18 @@
 from datetime import datetime, timezone
 from typing import TYPE_CHECKING, ClassVar
 
+from pydantic import field_validator
 from sift.assets.v1.assets_pb2 import Asset as AssetProto
 
 from sift_client.sift_types._base import BaseType, MappingHelper, ModelUpdate
 from sift_client.sift_types._mixins.file_attachments import FileAttachmentsMixin
 from sift_client.sift_types.tag import Tag
-from sift_client.util.metadata import Metadata, metadata_dict_to_proto, metadata_proto_to_dict
+from sift_client.util.metadata import (
+    Metadata,
+    expand_metadata_for_write,
+    metadata_dict_to_proto,
+    metadata_proto_to_dict,
+)
 
 if TYPE_CHECKING:
     from sift_client.client import SiftClient
@@ -114,9 +120,21 @@ class AssetUpdate(ModelUpdate[AssetProto]):
     """Model of the Asset Fields that can be updated."""
 
     tags: list[str | Tag] | None = None
-    metadata: dict[str, str | float | bool] | None = None
+    # A list[str] value writes every element under the same key (multi-value
+    # metadata, string keys only); writes REPLACE the full metadata map.
+    metadata: dict[str, str | float | bool | list[str]] | None = None
     is_archived: bool | None = None
 
+    @field_validator("metadata", mode="before")
+    @classmethod
+    def _expand_metadata(cls, value):
+        # A Metadata mapping hides values beyond the first behind getall();
+        # expand it before validation so pydantic's dict rebuild (and the
+        # model_dump that feeds proto conversion) can't drop them.
+        if isinstance(value, Metadata):
+            return expand_metadata_for_write(value)
+        return value
+
     _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = {
         "metadata": MappingHelper(
             proto_attr_path="metadata",

python/lib/sift_client/sift_types/run.py

@@ -3,7 +3,7 @@
 from datetime import datetime, timedelta, timezone
 from typing import TYPE_CHECKING, ClassVar
 
-from pydantic import model_validator
+from pydantic import field_validator, model_validator
 from sift.runs.v2.runs_pb2 import CreateRunRequest as CreateRunRequestProto
 from sift.runs.v2.runs_pb2 import Run as RunProto
 
@@ -16,7 +16,12 @@
 )
 from sift_client.sift_types._mixins.file_attachments import FileAttachmentsMixin
 from sift_client.sift_types.tag import Tag
-from sift_client.util.metadata import Metadata, metadata_dict_to_proto, metadata_proto_to_dict
+from sift_client.util.metadata import (
+    Metadata,
+    expand_metadata_for_write,
+    metadata_dict_to_proto,
+    metadata_proto_to_dict,
+)
 
 if TYPE_CHECKING:
     from pathlib import Path
@@ -178,7 +183,19 @@ class RunBase(ModelCreateUpdateBase):
     start_time: datetime | None = None
     stop_time: datetime | None = None
     tags: list[str] | list[Tag] | None = None
-    metadata: dict[str, str | float | bool] | None = None
+    # A list[str] value writes every element under the same key (multi-value
+    # metadata, string keys only); writes REPLACE the full metadata map.
+    metadata: dict[str, str | float | bool | list[str]] | None = None
+
+    @field_validator("metadata", mode="before")
+    @classmethod
+    def _expand_metadata(cls, value):
+        # A Metadata mapping hides values beyond the first behind getall();
+        # expand it before validation so pydantic's dict rebuild (and the
+        # model_dump that feeds proto conversion) can't drop them.
+        if isinstance(value, Metadata):
+            return expand_metadata_for_write(value)
+        return value
 
     _to_proto_helpers: ClassVar[dict[str, MappingHelper]] = {
         "metadata": MappingHelper(
@@ -233,6 +250,11 @@ def _to_rust_form(self) -> RunFormPy:
         if self.metadata:
             metadata = []
             for key, value in self.metadata.items():
+                if isinstance(value, list):
+                    raise ValueError(
+                        f"Metadata key '{key}': multi-value metadata is not supported "
+                        "on the streaming ingestion path yet; use a scalar value"
+                    )
                 metadata.append(MetadataPy(key=key, value=MetadataValuePy(value)))
         else:
             metadata = None