diff --git a/pinecone/_internal/validation.py b/pinecone/_internal/validation.py index fcab5d86..4bcce0a9 100644 --- a/pinecone/_internal/validation.py +++ b/pinecone/_internal/validation.py @@ -2,10 +2,52 @@ from __future__ import annotations -from collections.abc import Sequence -from typing import Any, overload +import functools +import inspect +import sys +from collections.abc import Callable, Sequence +from typing import Any, ParamSpec, TypeVar, overload + +from pinecone.errors.exceptions import PineconeTypeError, ValidationError + +_P = ParamSpec("_P") +_R = TypeVar("_R") + + +def reject_positional_args(example: str) -> Callable[[Callable[_P, _R]], Callable[_P, _R]]: + """Give a keyword-only instance method a teaching error on positional misuse. + + The interpreter's own failure ("takes 1 positional argument but N were + given") names neither the parameters nor the fix. This guard raises + :class:`PineconeTypeError` (a ``TypeError`` subclass) carrying *example*, + the exact call shape to use instead. The wrapper is a plain function even + for async methods, so the check runs at call time — before the coroutine + is created — matching native keyword-only binding rather than deferring + the error to await. ``inspect.signature`` still reports the wrapped + method's real keyword-only signature via ``__wrapped__``; on Python 3.12+ + an async method's wrapper is also marked for + ``inspect.iscoroutinefunction``. The bound ``self`` is the only + positional argument allowed through. + """ + + def decorate(fn: Callable[_P, _R]) -> Callable[_P, _R]: + message = ( + f"{fn.__name__}() accepts keyword arguments only: {example}. " + "Positional arguments are rejected because the parameter order " + "changed across SDK generations." + ) + + @functools.wraps(fn) + def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: + if len(args) > 1: + raise PineconeTypeError(message) + return fn(*args, **kwargs) + + if sys.version_info >= (3, 12) and inspect.iscoroutinefunction(fn): + inspect.markcoroutinefunction(wrapper) + return wrapper -from pinecone.errors.exceptions import ValidationError + return decorate @overload diff --git a/pinecone/async_client/async_index.py b/pinecone/async_client/async_index.py index 664a88d2..6fcb229b 100644 --- a/pinecone/async_client/async_index.py +++ b/pinecone/async_client/async_index.py @@ -23,7 +23,11 @@ _validate_host, _vector_to_dict, ) -from pinecone._internal.validation import require_in_range, require_positive +from pinecone._internal.validation import ( + reject_positional_args, + require_in_range, + require_positive, +) from pinecone._internal.vector_factory import VectorFactory from pinecone.errors.exceptions import PineconeValueError, ValidationError from pinecone.models.imports.list import ImportList @@ -138,6 +142,7 @@ def host(self) -> str: """The data plane host URL for this index.""" return self._host + @reject_positional_args('upsert_records(namespace="...", records=[...])') async def upsert_records( self, *, diff --git a/pinecone/grpc/__init__.py b/pinecone/grpc/__init__.py index 964ef422..9d60b8ac 100644 --- a/pinecone/grpc/__init__.py +++ b/pinecone/grpc/__init__.py @@ -19,7 +19,11 @@ from pinecone._internal.config import PineconeConfig from pinecone._internal.constants import DATA_PLANE_API_VERSION from pinecone._internal.data_plane_helpers import _build_search_records_body, _validate_host -from pinecone._internal.validation import require_in_range, require_positive +from pinecone._internal.validation import ( + reject_positional_args, + require_in_range, + require_positive, +) from pinecone._internal.vector_factory import VectorFactory from pinecone.errors.exceptions import ( PineconeValueError, @@ -1312,6 +1316,7 @@ def query_namespaces_async( ) ) + @reject_positional_args('upsert_records(namespace="...", records=[...])') def upsert_records( self, *, diff --git a/pinecone/index/__init__.py b/pinecone/index/__init__.py index 07b15bc6..b69e417c 100644 --- a/pinecone/index/__init__.py +++ b/pinecone/index/__init__.py @@ -22,7 +22,11 @@ _validate_host, _vector_to_dict, ) -from pinecone._internal.validation import require_in_range, require_positive +from pinecone._internal.validation import ( + reject_positional_args, + require_in_range, + require_positive, +) from pinecone._internal.vector_factory import VectorFactory from pinecone.errors.exceptions import PineconeValueError, ValidationError from pinecone.models.imports.list import ImportList @@ -465,6 +469,7 @@ def upsert_from_dataframe( timeout=timeout, ) + @reject_positional_args('upsert_records(namespace="...", records=[...])') def upsert_records( self, *, diff --git a/tests/unit/test_async_upsert_records.py b/tests/unit/test_async_upsert_records.py index 9e667c7f..46faef20 100644 --- a/tests/unit/test_async_upsert_records.py +++ b/tests/unit/test_async_upsert_records.py @@ -9,7 +9,7 @@ import respx from pinecone.async_client.async_index import AsyncIndex -from pinecone.errors.exceptions import ValidationError +from pinecone.errors.exceptions import PineconeTypeError, ValidationError from pinecone.models.vectors.responses import UpsertRecordsResponse INDEX_HOST = "my-index-abc123.svc.pinecone.io" @@ -140,3 +140,23 @@ async def test_async_upsert_records_id_must_be_string(self) -> None: namespace="test-ns", records=[{"_id": 123, "text": "hello"}], ) + + @pytest.mark.anyio + async def test_async_upsert_records_positional_args_raise_teaching_error(self) -> None: + """Positional misuse names the keyword-only call shape, not a bare TypeError.""" + idx = _make_async_index() + with pytest.raises(PineconeTypeError) as excinfo: + await idx.upsert_records("test-ns", [{"_id": "r1", "text": "hello"}]) # type: ignore[misc] + message = str(excinfo.value) + assert "keyword arguments only" in message + assert 'upsert_records(namespace="...", records=[...])' in message + + def test_async_upsert_records_positional_error_raises_at_call_time(self) -> None: + """The guard fires at the call, before any coroutine is created. + + A deferred (await-time) error would hand back a coroutine and a + 'never awaited' warning instead of the teaching message. + """ + idx = _make_async_index() + with pytest.raises(PineconeTypeError): + idx.upsert_records("test-ns", [{"_id": "r1"}]) # type: ignore[misc] diff --git a/tests/unit/test_grpc_index.py b/tests/unit/test_grpc_index.py index 37ebcf83..5ff2493a 100644 --- a/tests/unit/test_grpc_index.py +++ b/tests/unit/test_grpc_index.py @@ -18,6 +18,7 @@ NotFoundError, PineconeConnectionError, PineconeTimeoutError, + PineconeTypeError, ServiceError, UnauthorizedError, ValidationError, @@ -986,6 +987,17 @@ def test_no_double_bracket_in_propagated_not_found_error( class TestGrpcIndexUpsertRecords: """GrpcIndex.upsert_records() delegates to REST (NDJSON) endpoint.""" + def test_upsert_records_positional_args_raise_teaching_error( + self, mock_channel: MagicMock + ) -> None: + """Positional misuse names the keyword-only call shape, not a bare TypeError.""" + idx = _make_grpc_index(mock_channel, host=_INDEX_HOST) + with pytest.raises(PineconeTypeError) as excinfo: + idx.upsert_records("test-ns", [{"_id": "r1", "text": "hello"}]) # type: ignore[misc] + message = str(excinfo.value) + assert "keyword arguments only" in message + assert 'upsert_records(namespace="...", records=[...])' in message + @respx.mock def test_upsert_records_basic(self, mock_channel: MagicMock) -> None: respx.post(_UPSERT_RECORDS_URL).mock(return_value=httpx.Response(201)) diff --git a/tests/unit/test_upsert_records.py b/tests/unit/test_upsert_records.py index 8d7d6097..dc65ed66 100644 --- a/tests/unit/test_upsert_records.py +++ b/tests/unit/test_upsert_records.py @@ -2,6 +2,7 @@ from __future__ import annotations +import inspect import json import httpx @@ -9,7 +10,7 @@ import respx from pinecone import Index -from pinecone.errors.exceptions import ValidationError +from pinecone.errors.exceptions import PineconeTypeError, ValidationError from pinecone.models.vectors.responses import UpsertRecordsResponse INDEX_HOST = "my-index-abc123.svc.pinecone.io" @@ -169,6 +170,20 @@ def test_upsert_records_keyword_only(self) -> None: with pytest.raises(TypeError): idx.upsert_records([{"_id": "r1"}], "test-ns") # type: ignore[misc] + def test_upsert_records_positional_args_raise_teaching_error(self) -> None: + """Positional misuse names the keyword-only call shape, not a bare TypeError.""" + idx = _make_index() + with pytest.raises(PineconeTypeError) as excinfo: + idx.upsert_records("test-ns", [{"_id": "r1", "text": "hello"}]) # type: ignore[misc] + message = str(excinfo.value) + assert "keyword arguments only" in message + assert 'upsert_records(namespace="...", records=[...])' in message + + def test_upsert_records_signature_reports_keyword_only_params(self) -> None: + params = inspect.signature(Index.upsert_records).parameters + assert params["records"].kind is inspect.Parameter.KEYWORD_ONLY + assert params["namespace"].kind is inspect.Parameter.KEYWORD_ONLY + @respx.mock def test_upsert_records_preserves_metadata_fields(self) -> None: """Non-field_map fields (e.g. 'category') are forwarded in the NDJSON payload.