fix(index): raise ValidationError for invalid _id field in upsert_records

## Purpose

Two silent validation gaps caused confusing HTTP 400 errors from the backend:
1. Records with both `_id` and `id` were silently stripped instead of rejected.
2. Non-string `_id` values (e.g. `{"_id": 123}`) passed SDK validation but returned HTTP 400.

## Solution

In the normalization loop of `Index.upsert_records` and `AsyncIndex.upsert_records`:
- Changed the silent `del r["id"]` to raise `ValidationError` when both `_id` and `id`
  are present, matching the backend's `InvalidArgument` behavior.
- Added a type check after normalization: raises `ValidationError` if the resolved `_id`
  is not a `str`, catching integer and None IDs before the network call.

Updated unit tests (sync + async) and added integration-level validation tests.

Author: jhamon

pinecone/async_client/async_index.py

@@ -193,12 +193,16 @@ async def upsert_records(
         import orjson
 
         normalized: list[dict[str, Any]] = []
-        for record in records:
+        for i, record in enumerate(records):
             r = dict(record)  # shallow copy
             if "_id" not in r and "id" in r:
                 r["_id"] = r.pop("id")
             elif "_id" in r and "id" in r:
-                del r["id"]  # _id takes precedence; strip the extra key
+                raise ValidationError(f"Record at index {i} cannot have both '_id' and 'id' fields")
+            resolved_id = r.get("_id")
+            if not isinstance(resolved_id, str):
+                got = type(resolved_id).__name__
+                raise ValidationError(f"Record at index {i}: '_id' must be a string, got {got!r}")
             normalized.append(r)
 
         ndjson_lines = [orjson.dumps(r).decode("utf-8") for r in normalized]

pinecone/index/__init__.py

@@ -522,12 +522,16 @@ def upsert_records(
         import orjson
 
         normalized: list[dict[str, Any]] = []
-        for record in records:
+        for i, record in enumerate(records):
             r = dict(record)  # shallow copy
             if "_id" not in r and "id" in r:
                 r["_id"] = r.pop("id")
             elif "_id" in r and "id" in r:
-                del r["id"]  # _id takes precedence; strip the extra key
+                raise ValidationError(f"Record at index {i} cannot have both '_id' and 'id' fields")
+            resolved_id = r.get("_id")
+            if not isinstance(resolved_id, str):
+                got = type(resolved_id).__name__
+                raise ValidationError(f"Record at index {i}: '_id' must be a string, got {got!r}")
             normalized.append(r)
 
         ndjson_lines = [orjson.dumps(r).decode("utf-8") for r in normalized]

tests/integration/test_data_plane.py

@@ -12,6 +12,7 @@
 
 from pinecone import GrpcIndex, Index, Pinecone
 from pinecone.errors import ApiError, ConflictError, PineconeValueError
+from pinecone.errors.exceptions import ValidationError
 from pinecone.grpc.future import PineconeFuture
 from pinecone.models.indexes.specs import ServerlessSpec
 from pinecone.models.namespaces.models import ListNamespacesResponse, NamespaceDescription
@@ -2274,3 +2275,22 @@ async def test_async_search_with_sparse_vector() -> None:
     assert body["query"]["vector"] == {"sparse_indices": [10, 20], "sparse_values": [0.5, 0.3]}
     assert isinstance(response.result.hits, list)
     assert response.usage.read_units >= 0
+
+
+# ---------------------------------------------------------------------------
+# upsert_records — client-side ID validation
+# ---------------------------------------------------------------------------
+
+
+def test_upsert_records_id_must_be_string() -> None:
+    """upsert_records raises ValidationError when '_id' is not a string."""
+    idx = Index(host="my-index.svc.pinecone.io", api_key="test-key")
+    with pytest.raises(ValidationError, match="'_id' must be a string"):
+        idx.upsert_records(namespace="ns", records=[{"_id": 123, "text": "hello"}])
+
+
+def test_upsert_records_both_id_fields_rejected() -> None:
+    """upsert_records raises ValidationError when record has both '_id' and 'id' fields."""
+    idx = Index(host="my-index.svc.pinecone.io", api_key="test-key")
+    with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
+        idx.upsert_records(namespace="ns", records=[{"_id": "a", "id": "b", "text": "hello"}])

tests/unit/test_async_upsert_records.py

@@ -116,23 +116,22 @@ async def test_async_upsert_records_keyword_only(self) -> None:
         with pytest.raises(TypeError):
             await idx.upsert_records([{"_id": "r1"}], "ns")  # type: ignore[misc]
 
-    @respx.mock
     @pytest.mark.anyio
-    async def test_async_upsert_records_both_id_keys_underscore_wins(self) -> None:
-        """When both '_id' and 'id' are present, '_id' takes precedence and 'id' is stripped."""
-        route = respx.post(UPSERT_URL).mock(return_value=httpx.Response(201, content=b""))
+    async def test_async_upsert_records_both_id_fields_rejected(self) -> None:
+        """Records with both '_id' and 'id' fields raise ValidationError."""
         idx = _make_async_index()
-        result = await idx.upsert_records(
-            namespace="test-ns",
-            records=[
-                {"_id": "underscore-id-wins", "id": "plain-id-loses", "text": "both keys test"}
-            ],
-        )
-        assert isinstance(result, UpsertRecordsResponse)
-        assert result.record_count == 1
-        request = route.calls[0].request
-        body = request.content.decode("utf-8")
-        parsed = json.loads(body.strip())
-        assert parsed["_id"] == "underscore-id-wins"
-        assert "id" not in parsed
-        assert parsed["text"] == "both keys test"
+        with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
+            await idx.upsert_records(
+                namespace="test-ns",
+                records=[{"_id": "a", "id": "b", "text": "hello"}],
+            )
+
+    @pytest.mark.anyio
+    async def test_async_upsert_records_id_must_be_string(self) -> None:
+        """Records where '_id' is not a string raise ValidationError."""
+        idx = _make_async_index()
+        with pytest.raises(ValidationError, match="'_id' must be a string"):
+            await idx.upsert_records(
+                namespace="test-ns",
+                records=[{"_id": 123, "text": "hello"}],
+            )

tests/unit/test_upsert_records.py

@@ -139,26 +139,23 @@ def test_upsert_records_mixed_id_formats(self) -> None:
         assert parsed_1["_id"] == "r2"
         assert "id" not in parsed_1
 
-    @respx.mock
-    def test_upsert_records_both_id_keys_underscore_wins(self) -> None:
-        """When both '_id' and 'id' are present, '_id' takes precedence and 'id' is stripped."""
-        route = respx.post(UPSERT_URL).mock(
-            return_value=httpx.Response(201),
-        )
+    def test_upsert_records_both_id_fields_rejected(self) -> None:
+        """Records with both '_id' and 'id' fields raise ValidationError."""
         idx = _make_index()
-        result = idx.upsert_records(
-            namespace="test-ns",
-            records=[
-                {"_id": "underscore-id-wins", "id": "plain-id-loses", "text": "both keys test"}
-            ],
-        )
+        with pytest.raises(ValidationError, match="cannot have both '_id' and 'id'"):
+            idx.upsert_records(
+                namespace="test-ns",
+                records=[{"_id": "a", "id": "b", "text": "hello"}],
+            )
 
-        assert result.record_count == 1
-        body = route.calls.last.request.content.decode("utf-8")
-        parsed = json.loads(body.strip())
-        assert parsed["_id"] == "underscore-id-wins"
-        assert "id" not in parsed
-        assert parsed["text"] == "both keys test"
+    def test_upsert_records_id_must_be_string(self) -> None:
+        """Records where '_id' is not a string raise ValidationError."""
+        idx = _make_index()
+        with pytest.raises(ValidationError, match="'_id' must be a string"):
+            idx.upsert_records(
+                namespace="test-ns",
+                records=[{"_id": 123, "text": "hello"}],
+            )
 
     def test_upsert_records_keyword_only(self) -> None:
         idx = _make_index()