From e31afa7bf3d2e5adf647f0e92871241247a09a79 Mon Sep 17 00:00:00 2001 From: joerg84 Date: Fri, 17 Jul 2026 08:56:59 -0400 Subject: [PATCH 1/2] fix(index): teach the keyword-only call shape when upsert_records gets positional args upsert_records(ns, records) previously failed with the interpreter's bare 'takes 1 positional argument but 3 were given', which names neither the parameters nor the fix. The keyword-only contract is deliberate (parameter order changed across SDK generations, so positionals would silently mis-bind); the failure just needs to say so. Add a reject_positional_args guard that raises PineconeTypeError (a TypeError subclass) carrying the exact call shape, applied to the sync, async, and gRPC variants. inspect.signature still reports the real keyword-only signature via __wrapped__. Fixes #691 Co-Authored-By: Claude Fable 5 --- pinecone/_internal/validation.py | 50 +++++++++++++++++++++++-- pinecone/async_client/async_index.py | 7 +++- pinecone/grpc/__init__.py | 7 +++- pinecone/index/__init__.py | 7 +++- tests/unit/test_async_upsert_records.py | 12 +++++- tests/unit/test_grpc_index.py | 12 ++++++ tests/unit/test_upsert_records.py | 17 ++++++++- 7 files changed, 104 insertions(+), 8 deletions(-) diff --git a/pinecone/_internal/validation.py b/pinecone/_internal/validation.py index fcab5d86..e0f29e34 100644 --- a/pinecone/_internal/validation.py +++ b/pinecone/_internal/validation.py @@ -2,10 +2,54 @@ from __future__ import annotations -from collections.abc import Sequence -from typing import Any, overload +import functools +import inspect +from collections.abc import Awaitable, Callable, Sequence +from typing import Any, ParamSpec, TypeVar, cast, overload -from pinecone.errors.exceptions import ValidationError +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. ``inspect.signature`` still reports + the wrapped method's real keyword-only signature via ``__wrapped__``. + 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." + ) + if inspect.iscoroutinefunction(fn): + async_fn = cast("Callable[_P, Awaitable[_R]]", fn) + + @functools.wraps(fn) + async def async_wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: + if len(args) > 1: + raise PineconeTypeError(message) + return await async_fn(*args, **kwargs) + + return cast("Callable[_P, _R]", async_wrapper) + + @functools.wraps(fn) + def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: + if len(args) > 1: + raise PineconeTypeError(message) + return fn(*args, **kwargs) + + return wrapper + + 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..26e247d5 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,13 @@ 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 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. From 3ac800ea8b30b9bf1332d81883d7da8c3c1305c5 Mon Sep 17 00:00:00 2001 From: joerg84 Date: Fri, 17 Jul 2026 10:30:29 -0400 Subject: [PATCH 2/2] fix: raise the positional-args teaching error at call time for async methods The async branch wrapped with 'async def', deferring the guard to await time - an un-awaited misuse returned a coroutine and a 'never awaited' warning instead of the error, where native keyword-only binding raises at the call. Use one plain wrapper for both cases (returning the coroutine for async methods) so the check runs at call time, and mark the wrapper via inspect.markcoroutinefunction on 3.12+ so iscoroutinefunction still reports correctly. Flagged by Cursor Bugbot. Co-Authored-By: Claude Fable 5 --- pinecone/_internal/validation.py | 28 ++++++++++++------------- tests/unit/test_async_upsert_records.py | 10 +++++++++ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/pinecone/_internal/validation.py b/pinecone/_internal/validation.py index e0f29e34..4bcce0a9 100644 --- a/pinecone/_internal/validation.py +++ b/pinecone/_internal/validation.py @@ -4,8 +4,9 @@ import functools import inspect -from collections.abc import Awaitable, Callable, Sequence -from typing import Any, ParamSpec, TypeVar, cast, overload +import sys +from collections.abc import Callable, Sequence +from typing import Any, ParamSpec, TypeVar, overload from pinecone.errors.exceptions import PineconeTypeError, ValidationError @@ -19,9 +20,14 @@ def reject_positional_args(example: str) -> Callable[[Callable[_P, _R]], Callabl 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. ``inspect.signature`` still reports - the wrapped method's real keyword-only signature via ``__wrapped__``. - The bound ``self`` is the only positional argument allowed through. + 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]: @@ -30,16 +36,6 @@ def decorate(fn: Callable[_P, _R]) -> Callable[_P, _R]: "Positional arguments are rejected because the parameter order " "changed across SDK generations." ) - if inspect.iscoroutinefunction(fn): - async_fn = cast("Callable[_P, Awaitable[_R]]", fn) - - @functools.wraps(fn) - async def async_wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: - if len(args) > 1: - raise PineconeTypeError(message) - return await async_fn(*args, **kwargs) - - return cast("Callable[_P, _R]", async_wrapper) @functools.wraps(fn) def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: @@ -47,6 +43,8 @@ def wrapper(*args: _P.args, **kwargs: _P.kwargs) -> _R: raise PineconeTypeError(message) return fn(*args, **kwargs) + if sys.version_info >= (3, 12) and inspect.iscoroutinefunction(fn): + inspect.markcoroutinefunction(wrapper) return wrapper return decorate diff --git a/tests/unit/test_async_upsert_records.py b/tests/unit/test_async_upsert_records.py index 26e247d5..46faef20 100644 --- a/tests/unit/test_async_upsert_records.py +++ b/tests/unit/test_async_upsert_records.py @@ -150,3 +150,13 @@ async def test_async_upsert_records_positional_args_raise_teaching_error(self) - 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]