From 5b4836475bf1c124d91dac6ceed75aef0f5722eb Mon Sep 17 00:00:00 2001 From: Minseok Park Date: Thu, 1 Oct 2026 15:28:46 +0900 Subject: [PATCH 1/4] fix: refuse bool and float item IDs everywhere instead of coercing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete, update_metadata, update_documents and upsert_documents turned each id into int(...), and so did the ids of add_texts / add_documents. int(True) is 1 and int(3.9) is 3, so delete(ids=[3.9]) deleted item 3 and add_texts(..., ids=[3.9]) overwrote it — rows the caller never named. All of them now share _readable_item_id, the check get_by_ids uses: only a positive int or its ASCII decimal string is an item ID. What happens to anything else follows each method's existing rule — ValueError from delete / update_* / upsert_documents, a new row plus a UserWarning for the ids of add_texts. Non-positive ids keep their own "positive integers" message. The check also stops accepting non-ASCII digits ("٣" read item 3 through str.isdecimal), which get_by_ids did. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../langchain_envector/vectorstore.py | 126 ++++++++++-------- tests/test_vectorstore.py | 75 +++++++++++ 2 files changed, 144 insertions(+), 57 deletions(-) diff --git a/libs/envector/langchain_envector/vectorstore.py b/libs/envector/langchain_envector/vectorstore.py index 0019d11..881c689 100644 --- a/libs/envector/langchain_envector/vectorstore.py +++ b/libs/envector/langchain_envector/vectorstore.py @@ -17,26 +17,70 @@ SDK_HAS_GET_BY_IDS = hasattr(_SdkIndex, "get_by_ids") +# Item IDs travel as proto int64; a larger value cannot name a row and the +# SDK would fail to encode it. +_MAX_ITEM_ID = 2**63 - 1 + + +def _readable_item_id(value: Any) -> Optional[int]: + """The item ID ``value`` names exactly, or ``None`` when it names none. + + The one ID check every method shares: only a positive ``int`` within int64 + (the server issues item IDs as ``int64``) or an ASCII decimal string of one + counts. ``bool`` and ``float`` are not item IDs — ``int(True)`` is 1 and + ``int(3.9)`` is 3, so coercing them would address a different document — + and neither are non-ASCII digits such as ``"٣"`` or ``"３"``, which + ``str.isdecimal`` accepts. + """ + if isinstance(value, bool): + return None + if isinstance(value, int): + return value if 0 < value <= _MAX_ITEM_ID else None + if isinstance(value, str): + text = value.strip() + if text.isascii() and text.isdigit(): + item_id = int(text) + return item_id if 0 < item_id <= _MAX_ITEM_ID else None + return None + + +def _is_non_positive_integer(value: Any) -> bool: + """True for an integer (or its ASCII decimal string, sign allowed) <= 0.""" + if isinstance(value, bool): + return False + if isinstance(value, int): + return value <= 0 + if isinstance(value, str): + text = value.strip() + digits = text[1:] if text[:1] in "+-" else text + return bool(digits) and digits.isascii() and digits.isdigit() and int(text) <= 0 + return False + + def _mutation_items( item_ids: List[Any], label: str, *, dedupe: bool = False ) -> List[int]: - """Coerce caller-supplied IDs to the ``int`` item_ids the SDK addresses. + """Turn caller-supplied IDs into the ``int`` item_ids the SDK addresses. - The SDK rejects non-positive and repeated ids with its own message; both - are caught here and named after the calling method. ``dedupe=True`` - (delete) drops repeats instead, since deleting a row twice is deleting it. + Used by the methods that change rows (delete, update, upsert), so a value + that names no item raises instead of being coerced onto another item — see + `_readable_item_id`. Non-positive and repeated ids are named after the + calling method. ``dedupe=True`` (delete) drops repeats instead, since + deleting a row twice is deleting it. """ - try: - ints = [int(x) for x in item_ids] - except (TypeError, ValueError) as e: - raise ValueError( - f"Envector.{label} expects integer item IDs (or numeric strings) " - "as returned by add_texts/add_documents." - ) from e - if any(i <= 0 for i in ints): - raise ValueError( - f"Envector.{label}: item IDs are positive integers (got {min(ints)})." - ) + ints: List[int] = [] + for x in item_ids: + item_id = _readable_item_id(x) + if item_id is None: + if _is_non_positive_integer(x): + raise ValueError( + f"Envector.{label}: item IDs are positive integers (got {x!r})." + ) + raise ValueError( + f"Envector.{label} expects integer item IDs (a positive int or its " + f"decimal string) as returned by add_texts/add_documents; got {x!r}." + ) + ints.append(item_id) if dedupe: return list(dict.fromkeys(ints)) if len(set(ints)) != len(ints): @@ -48,9 +92,9 @@ def _split_caller_ids(ids: List[Any]) -> Tuple[List[Optional[int]], List[Any]]: """Sort caller-supplied IDs into enVector item IDs and everything else. Returns ``(item_ids, foreign)``: ``item_ids`` is positional against ``ids`` - with ``None`` wherever the entry was ``None`` or not an integer, and - ``foreign`` lists the non-integer values so the caller can be told they - were not honoured. + with ``None`` wherever the entry was ``None`` or names no item (see + `_readable_item_id`), and ``foreign`` lists those other values so the + caller can be told they were not honoured. """ item_ids: List[Optional[int]] = [] foreign: List[Any] = [] @@ -58,46 +102,13 @@ def _split_caller_ids(ids: List[Any]) -> Tuple[List[Optional[int]], List[Any]]: if x is None: item_ids.append(None) continue - try: - value = int(x) - except (TypeError, ValueError): - value = 0 - if value <= 0: # the server issues positive ints only - item_ids.append(None) + value = _readable_item_id(x) + item_ids.append(value) + if value is None: foreign.append(x) - else: - item_ids.append(value) return item_ids, foreign -# Item IDs travel as proto int64; a larger value cannot name a row and the -# SDK would fail to encode it. -_MAX_ITEM_ID = 2**63 - 1 - - -def _readable_item_id(value: Any) -> Optional[int]: - """The item ID ``value`` names exactly, or ``None`` when it names none. - - For `get_by_ids`, which must never read an item the caller did not name: - only a positive ``int`` within int64 (the server issues item IDs as - ``int64``), or an ASCII decimal string of one, counts. ``bool`` and - ``float`` are not item IDs — ``int(True)`` is 1 and ``int(3.9)`` is 3, so - coercing them would return a different document — and neither are - non-ASCII digits such as ``"٣"`` or ``"３"``, which ``str.isdecimal`` - accepts. - """ - if isinstance(value, bool): - return None - if isinstance(value, int): - return value if 0 < value <= _MAX_ITEM_ID else None - if isinstance(value, str): - text = value.strip() - if text.isascii() and text.isdigit(): - item_id = int(text) - return item_id if 0 < item_id <= _MAX_ITEM_ID else None - return None - - def _one_embedding_arg(embedding: Any, embeddings: Any) -> Any: """Resolve the standard positional ``embedding`` and our older ``embeddings=`` keyword into one value, rejecting conflicting pairs.""" @@ -279,7 +290,7 @@ def add_texts( not affected. ``ids`` follows LangChain's add-or-update contract as far as enVector - allows: an entry that is an item ID (int or numeric str, such as the + allows: an entry that is an item ID (a positive int or its decimal str, such as the ``Document.id`` search results carry) updates that item in place; an ID with no live row, or a non-integer ID, cannot be created, so that row is inserted with a server-issued ID and a ``UserWarning``. ``None`` entries @@ -431,8 +442,9 @@ def delete( """Delete items from the encrypted index by item ID. Accepts the ``item_id`` values returned from ``add_texts`` / - ``add_documents``. Both ``int`` and ``str`` (numeric) IDs are accepted - and coerced to ``int`` before being passed to the SDK. + ``add_documents``, as ``str`` or ``int``. Any other value — ``bool`` and + ``float`` included — raises ``ValueError`` rather than being coerced onto + another item (``3.9`` would otherwise delete item 3). Deletion is asynchronous server-side; by default this waits until the affected shards are rebuilt and the remaining data is searchable again diff --git a/tests/test_vectorstore.py b/tests/test_vectorstore.py index 8cbd299..c9ae71e 100644 --- a/tests/test_vectorstore.py +++ b/tests/test_vectorstore.py @@ -1490,3 +1490,78 @@ def test_search_skips_a_hit_with_empty_text_and_metadata(): ] store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) assert [d.id for d in store.similarity_search("q", k=2)] == ["2"] + + +# Values that look like item IDs to int(...) but name none: True -> 1, +# 3.9 / 3.0 -> 3, "٣" (Arabic-Indic three) -> 3. None may address an item. +_COERCIBLE_NON_IDS = [True, 3.9, 3.0, "٣"] + + +@pytest.mark.parametrize("not_an_id", _COERCIBLE_NON_IDS) +def test_delete_refuses_values_that_name_no_item(not_an_id): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete([not_an_id]) + assert client.index.deleted == [] + + +@pytest.mark.parametrize("not_an_id", _COERCIBLE_NON_IDS) +def test_update_and_upsert_refuse_values_that_name_no_item(not_an_id): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + doc = LC_Document(page_content="new") + + with pytest.raises(ValueError, match="expects integer item IDs"): + store.update_metadata([not_an_id], ["new"]) + with pytest.raises(ValueError, match="expects integer item IDs"): + store.update_documents([not_an_id], [doc]) + with pytest.raises(ValueError, match="expects integer item IDs"): + store.upsert_documents([doc], ids=[not_an_id]) + assert client.index.updates == [] + assert client.index.upserts == [] + + +@pytest.mark.parametrize("not_an_id", _COERCIBLE_NON_IDS) +def test_add_texts_inserts_instead_of_overwriting_on_a_non_id(not_an_id): + # add_texts keeps its existing rule for ids it cannot honour: insert as a + # new row and warn. It must not update item 3 in place. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + + with pytest.warns(UserWarning, match="not enVector item IDs"): + new_ids = store.add_texts(["x"], ids=[not_an_id]) + assert new_ids == ["4"] + assert client.index.upserts == [] + assert client.index.stored[(None, 3)] == '{"text": "c", "metadata": {}}' + + +def test_mutations_still_take_ints_and_decimal_strings(): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c", "d"]) + + store.delete(["1", " 2 ", 3]) + assert client.index.deleted[-1]["item_ids"] == [1, 2, 3] + store.update_metadata([4], ["d2"]) + assert [it.item_id for it in client.index.updates[-1]["items"]] == [4] + + +def test_non_positive_ids_keep_their_own_message(): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + for bad in (0, -3, "-3", "0"): + with pytest.raises(ValueError, match="positive integers"): + store.delete([bad]) + + +def test_get_by_ids_ignores_non_ascii_digits(): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + assert store.get_by_ids(["٣"]) == [] + assert client.index.fetched == [] From 03c4e6f9d79bbad82d3018e174325ef74dcc41f7 Mon Sep 17 00:00:00 2001 From: Minseok Park Date: Thu, 1 Oct 2026 15:44:22 +0900 Subject: [PATCH 2/4] fix: accept NumPy integers as item IDs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shared ID check accepted only Python int, so an ID taken from a NumPy array or a pandas column — np.int64(3) — was refused: delete([np.int64(3)]) raised ValueError and add_texts inserted a new row instead of updating item 3, where main had treated it as item 3. Check for numbers.Integral (minus bool) instead. NumPy integers are registered as Integral; np.bool_ and np.float64 are not, so they stay refused like bool and float. The ID passed on is a plain int. Also rewrap an add_texts docstring line that ran past the line length. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../langchain_envector/vectorstore.py | 25 +++++++++++-------- tests/test_vectorstore.py | 23 +++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/libs/envector/langchain_envector/vectorstore.py b/libs/envector/langchain_envector/vectorstore.py index 881c689..895ad4e 100644 --- a/libs/envector/langchain_envector/vectorstore.py +++ b/libs/envector/langchain_envector/vectorstore.py @@ -1,5 +1,6 @@ from __future__ import annotations +import numbers import warnings from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple @@ -25,17 +26,18 @@ def _readable_item_id(value: Any) -> Optional[int]: """The item ID ``value`` names exactly, or ``None`` when it names none. - The one ID check every method shares: only a positive ``int`` within int64 - (the server issues item IDs as ``int64``) or an ASCII decimal string of one - counts. ``bool`` and ``float`` are not item IDs — ``int(True)`` is 1 and - ``int(3.9)`` is 3, so coercing them would address a different document — - and neither are non-ASCII digits such as ``"٣"`` or ``"３"``, which - ``str.isdecimal`` accepts. + The one ID check every method shares: only a positive integer within int64 + — any ``numbers.Integral``, so NumPy integers count; the server issues item + IDs as ``int64`` — or an ASCII decimal string of one counts. ``bool`` and + ``float`` are not item IDs — ``int(True)`` is 1 and ``int(3.9)`` is 3, so + coercing them would address a different document — and neither are + non-ASCII digits such as ``"٣"`` or ``"３"``, which ``str.isdecimal`` + accepts. """ if isinstance(value, bool): return None - if isinstance(value, int): - return value if 0 < value <= _MAX_ITEM_ID else None + if isinstance(value, numbers.Integral): + return int(value) if 0 < value <= _MAX_ITEM_ID else None if isinstance(value, str): text = value.strip() if text.isascii() and text.isdigit(): @@ -48,7 +50,7 @@ def _is_non_positive_integer(value: Any) -> bool: """True for an integer (or its ASCII decimal string, sign allowed) <= 0.""" if isinstance(value, bool): return False - if isinstance(value, int): + if isinstance(value, numbers.Integral): return value <= 0 if isinstance(value, str): text = value.strip() @@ -290,8 +292,9 @@ def add_texts( not affected. ``ids`` follows LangChain's add-or-update contract as far as enVector - allows: an entry that is an item ID (a positive int or its decimal str, such as the - ``Document.id`` search results carry) updates that item in place; an ID + allows: an entry that is an item ID (a positive int or its decimal str, + such as the ``Document.id`` search results carry) updates that item in + place; an ID with no live row, or a non-integer ID, cannot be created, so that row is inserted with a server-issued ID and a ``UserWarning``. ``None`` entries insert. The returned list holds the IDs actually in the index, as diff --git a/tests/test_vectorstore.py b/tests/test_vectorstore.py index c9ae71e..743a23f 100644 --- a/tests/test_vectorstore.py +++ b/tests/test_vectorstore.py @@ -1565,3 +1565,26 @@ def test_get_by_ids_ignores_non_ascii_digits(): store.add_texts(["a", "b", "c"]) assert store.get_by_ids(["٣"]) == [] assert client.index.fetched == [] + + +def test_numpy_integers_are_item_ids_and_numpy_bool_float_are_not(): + # IDs often come out of NumPy arrays or pandas columns. A NumPy integer is + # an integer; np.bool_ and np.float64 are not item IDs any more than their + # Python counterparts. + np = pytest.importorskip("numpy") + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c", "d"]) + + for good in (np.int64(3), np.int32(3), np.uint64(3)): + assert [d.page_content for d in store.get_by_ids([good])] == ["c"] + store.delete([np.int64(2)]) + assert client.index.deleted[-1]["item_ids"] == [2] + assert type(client.index.deleted[-1]["item_ids"][0]) is int + + for bad in (np.bool_(True), np.float64(3.9), np.float64(3.0)): + assert store.get_by_ids([bad]) == [] + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete([bad]) + with pytest.raises(ValueError, match="positive integers"): + store.delete([np.int64(0)]) From 9fc0667551a20482c273aeb07d654c4229a26961 Mon Sep 17 00:00:00 2001 From: Minseok Park Date: Fri, 2 Oct 2026 16:22:02 +0900 Subject: [PATCH 3/4] fix: refuse a bare string as ids, cap digit strings, and raise on wrong-typed ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #22 and #23 found four more ways an id could address a row the caller never named, or fail in the wrong place: - ids passed as a bare str or bytes iterated by element: delete("13") deleted items 1 and 3, delete(b"13") items 49 and 51. Every method that takes ids now refuses str / bytes / bytearray with a TypeError that says to pass a list, e.g. [doc.id]; tuples, sets and generators still work. The README delete example passed doc.id bare; it now passes [doc.id]. Pre-existing on main and in LangChain's own InMemoryVectorStore, but it is exactly what this PR exists to stop. - A digit string longer than 4300 characters made CPython's int() raise before any of our rules ran. A 19-digit cap (the length of 2**63 - 1) classifies such strings as "not an item ID" first. - add_texts inserted a new row with a warning for 3.0, True or np.float64(3.0). Those are wrongly-typed item IDs, not foreign IDs — a pandas integer column turns float64 on its first NaN — and a silent new row duplicates the document on every re-load. add_texts now raises for values that can be no ID at all (bool, float and other non-integral numbers, bytes, __index__ objects). Strings that are not item IDs ("0", "doc-7", a UUID, "+3", non-ASCII digits) still insert with a warning, since LangChain IDs are strings and 0-based chunk numbers are a common foreign ID. - _is_non_positive_integer parsed signs its own way, so "+0" and "+3" got different messages. One parser, _parse_integer, now feeds both _readable_item_id and _mutation_items: a leading "-" is read so 0 and negatives get the "positive integers" message, "+3" is simply not an item ID. Co-Authored-By: Claude Fable 5.1 --- README.md | 5 +- .../langchain_envector/vectorstore.py | 142 +++++++++++++----- tests/test_vectorstore.py | 130 +++++++++++++++- 3 files changed, 230 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 9b378b1..e7f4859 100644 --- a/README.md +++ b/README.md @@ -41,7 +41,8 @@ Key dataclasses live in `libs/envector/config.py`: - Client-side filtering requires the JSON envelope to include an object under `metadata`. ## Limitations -- Item IDs are issued by the server (positive integers, returned as strings). Pass them back — as those strings or as ints — to `get_by_ids`, `delete`, `update_documents`, `upsert_documents`, or as `ids` to `add_documents` to update in place. Other IDs, such as UUIDs, cannot be created; such rows get server-issued IDs and a `UserWarning`, and `get_by_ids` never finds them. +- Item IDs are issued by the server (positive integers, returned as strings). Pass them back in a list — as those strings or as ints, e.g. `[doc.id]` — to `get_by_ids`, `delete`, `update_documents`, `upsert_documents`, or as `ids` to `add_documents` to update in place. Other IDs, such as UUIDs, cannot be created; such rows get server-issued IDs and a `UserWarning`, and `get_by_ids` never finds them. +- Only a positive integer, or its plain decimal string, is an item ID. Anything else — a float such as `3.0`, a bool, `"+3"`, `"3_000"`, non-ASCII digits — names no item: `get_by_ids` skips it and `delete` / `update_*` / `upsert_documents` raise `ValueError`. In `add_documents`, a string of your own is inserted as a new row with a `UserWarning`, while a float or bool raises rather than inserting a duplicate. A bare string instead of a list raises `TypeError`. - LangChain's `indexing` API is unsupported, since it depends on its own IDs. - Item IDs are unique within a partition only; pass `partition_name` to `get_by_ids` for documents added to a named partition. - `get_by_ids` needs a pyenvector release that provides `Index.get_by_ids`; with an earlier pyenvector it raises `NotImplementedError`. @@ -189,7 +190,7 @@ docs = store.get_by_ids(ids, partition_name="tenant_a") # rows in a named parti ### Delete ```python -store.delete(ids) # accepts ints or numeric strings, e.g. doc.id +store.delete([doc.id]) # a list of item IDs, as ints or the strings add_documents returned ``` Deletion is asynchronous server-side; by default `delete` waits for the SDK's diff --git a/libs/envector/langchain_envector/vectorstore.py b/libs/envector/langchain_envector/vectorstore.py index 895ad4e..ed46d03 100644 --- a/libs/envector/langchain_envector/vectorstore.py +++ b/libs/envector/langchain_envector/vectorstore.py @@ -19,46 +19,88 @@ # Item IDs travel as proto int64; a larger value cannot name a row and the -# SDK would fail to encode it. +# SDK would fail to encode it. 19 digits is the length of 2**63 - 1, so a +# longer digit string is refused before int() sees it (CPython refuses to +# convert more than 4300 digits anyway, with its own error). _MAX_ITEM_ID = 2**63 - 1 +_MAX_ITEM_ID_DIGITS = 19 -def _readable_item_id(value: Any) -> Optional[int]: - """The item ID ``value`` names exactly, or ``None`` when it names none. +def _parse_integer(value: Any) -> Optional[int]: + """``value`` as the integer it spells, or ``None`` when it spells none. - The one ID check every method shares: only a positive integer within int64 - — any ``numbers.Integral``, so NumPy integers count; the server issues item - IDs as ``int64`` — or an ASCII decimal string of one counts. ``bool`` and - ``float`` are not item IDs — ``int(True)`` is 1 and ``int(3.9)`` is 3, so - coercing them would address a different document — and neither are - non-ASCII digits such as ``"٣"`` or ``"３"``, which ``str.isdecimal`` - accepts. + Accepts any ``numbers.Integral`` except ``bool`` (NumPy integers count), + and a string of at most 19 ASCII digits with an optional leading ``-``. + ``bool``, ``float``, ``"+3"``, ``"3_000"``, non-ASCII digits such as + ``"٣"`` or ``"３"``, and anything else spell no integer here: coercing them + with ``int()`` would address a document the caller did not name. """ if isinstance(value, bool): return None if isinstance(value, numbers.Integral): - return int(value) if 0 < value <= _MAX_ITEM_ID else None + return int(value) if isinstance(value, str): text = value.strip() - if text.isascii() and text.isdigit(): - item_id = int(text) - return item_id if 0 < item_id <= _MAX_ITEM_ID else None + digits = text[1:] if text[:1] == "-" else text + if ( + 0 < len(digits) <= _MAX_ITEM_ID_DIGITS + and digits.isascii() + and digits.isdigit() + ): + return int(text) return None -def _is_non_positive_integer(value: Any) -> bool: - """True for an integer (or its ASCII decimal string, sign allowed) <= 0.""" - if isinstance(value, bool): - return False +def _readable_item_id(value: Any) -> Optional[int]: + """The item ID ``value`` names exactly, or ``None`` when it names none. + + The one ID check every method shares: an integer per `_parse_integer` + that is positive and within int64, since the server issues item IDs as + ``int64``. + """ + item_id = _parse_integer(value) + if item_id is None or not 0 < item_id <= _MAX_ITEM_ID: + return None + return item_id + + +def _wrong_type_for_id(value: Any) -> bool: + """True when ``value`` cannot be an ID of any kind: a ``bool``, a ``float`` + or other non-integral number, bytes, or an ``__index__`` object such as a + 0-d NumPy array. + + LangChain IDs are strings, so a string that is not one of our item IDs + (a UUID, a 0-based chunk number) is a foreign ID and `add_texts` inserts + it as a new row with a warning. A ``float`` such as ``3.0`` is no ID at + all — it is almost always a wrongly-typed item ID, as when a pandas + integer column turns float64 on its first NaN — and a silent new row + would duplicate the document on every re-load, so `add_texts` raises. + """ + if isinstance(value, (bool, bytes, bytearray)): + return True if isinstance(value, numbers.Integral): - return value <= 0 - if isinstance(value, str): - text = value.strip() - digits = text[1:] if text[:1] in "+-" else text - return bool(digits) and digits.isascii() and digits.isdigit() and int(text) <= 0 + return False + if isinstance(value, numbers.Number) or hasattr(value, "__index__"): + return True return False +def _ids_list(ids: Any, label: str) -> List[Any]: + """``ids`` as a list, refusing a bare string or bytes. + + A ``str`` is itself a sequence of one-character strings, so + ``delete("13")`` would otherwise delete items 1 and 3, and ``bytes`` + iterate as ints — ``delete(b"13")`` would delete items 49 and 51. + """ + if isinstance(ids, (str, bytes, bytearray)): + raise TypeError( + f"Envector.{label}: ids must be a list of item IDs, e.g. [doc.id]; " + f"got a bare {type(ids).__name__}, which would be read one character " + "at a time." + ) + return list(ids) + + def _mutation_items( item_ids: List[Any], label: str, *, dedupe: bool = False ) -> List[int]: @@ -72,17 +114,18 @@ def _mutation_items( """ ints: List[int] = [] for x in item_ids: - item_id = _readable_item_id(x) - if item_id is None: - if _is_non_positive_integer(x): - raise ValueError( - f"Envector.{label}: item IDs are positive integers (got {x!r})." - ) + parsed = _parse_integer(x) + if parsed is None: raise ValueError( f"Envector.{label} expects integer item IDs (a positive int or its " f"decimal string) as returned by add_texts/add_documents; got {x!r}." ) - ints.append(item_id) + if not 0 < parsed <= _MAX_ITEM_ID: + raise ValueError( + f"Envector.{label}: item IDs are positive integers up to " + f"{_MAX_ITEM_ID} (got {x!r})." + ) + ints.append(parsed) if dedupe: return list(dict.fromkeys(ints)) if len(set(ints)) != len(ints): @@ -96,7 +139,9 @@ def _split_caller_ids(ids: List[Any]) -> Tuple[List[Optional[int]], List[Any]]: Returns ``(item_ids, foreign)``: ``item_ids`` is positional against ``ids`` with ``None`` wherever the entry was ``None`` or names no item (see `_readable_item_id`), and ``foreign`` lists those other values so the - caller can be told they were not honoured. + caller can be told they were not honoured. A value of a type that can be + no ID at all (see `_wrong_type_for_id`) raises instead: it is a + wrongly-typed item ID, not a foreign ID, and must not become a new row. """ item_ids: List[Optional[int]] = [] foreign: List[Any] = [] @@ -105,6 +150,13 @@ def _split_caller_ids(ids: List[Any]) -> Tuple[List[Optional[int]], List[Any]]: item_ids.append(None) continue value = _readable_item_id(x) + if value is None and _wrong_type_for_id(x): + raise ValueError( + f"Envector.add_texts: {x!r} is not an item ID. Item IDs are the " + "positive integers add_texts/add_documents return (as int or " + "decimal str); pass one of those to update in place, a string " + "of your own to insert under a server-issued ID, or None." + ) item_ids.append(value) if value is None: foreign.append(x) @@ -294,10 +346,13 @@ def add_texts( ``ids`` follows LangChain's add-or-update contract as far as enVector allows: an entry that is an item ID (a positive int or its decimal str, such as the ``Document.id`` search results carry) updates that item in - place; an ID - with no live row, or a non-integer ID, cannot be created, so that row is - inserted with a server-issued ID and a ``UserWarning``. ``None`` entries - insert. The returned list holds the IDs actually in the index, as + place; an ID with no live row, or an ID of another kind (a UUID, a + slug), cannot be created, so that row is inserted with a server-issued + ID and a ``UserWarning``. ``None`` entries insert. A value that can be + no ID at all — a ``float`` such as ``3.0``, a ``bool``, bytes — is a + wrongly-typed item ID, not a foreign one, and raises ``ValueError`` + instead of becoming a new row. ``ids`` must be a list; a bare string + raises ``TypeError``. The returned list holds the IDs actually in the index, as strings like LangChain's ``Document.id``; every method here accepts them back in that form. """ @@ -320,9 +375,10 @@ def add_texts( poll_interval_s = kwargs.pop("poll_interval_s", None) if ids is not None: + ids = _ids_list(ids, "add_texts") if len(ids) != len(texts): raise ValueError("texts and ids must have equal length") - item_ids, foreign = _split_caller_ids(list(ids)) + item_ids, foreign = _split_caller_ids(ids) if foreign: warnings.warn( f"Envector cannot insert under caller-chosen IDs; {len(foreign)} " @@ -458,6 +514,8 @@ def delete( applies the delete when the row lands (late-binding through InsertShardMapList), so deleting an unmerged row is safe. """ + if ids is not None: + ids = _ids_list(ids, "delete") if not ids: return False item_ids = _mutation_items(ids, "delete", dedupe=True) @@ -509,7 +567,9 @@ def get_by_ids( ) item_ids = list( dict.fromkeys( - i for i in (_readable_item_id(x) for x in ids) if i is not None + i + for i in (_readable_item_id(x) for x in _ids_list(ids, "get_by_ids")) + if i is not None ) ) if not item_ids: @@ -551,6 +611,7 @@ def update_metadata( matched no live row (missing or already deleted) — those are reported, not raised. """ + ids = _ids_list(ids, "update_metadata") if not ids: return {"request_id": [], "not_found_item_ids": []} if len(texts) != len(ids): @@ -559,7 +620,7 @@ def update_metadata( metadatas = [{} for _ in texts] if len(metadatas) != len(texts): raise ValueError("texts and metadatas must have equal length") - item_ids = _mutation_items(list(ids), "update_metadata") + item_ids = _mutation_items(ids, "update_metadata") packed = [pack_metadata(t, m) for t, m in zip(texts, metadatas)] return self._update_items( @@ -595,6 +656,7 @@ def update_documents( texts = [getattr(d, "page_content", "") for d in documents] metadatas = [getattr(d, "metadata", {}) for d in documents] + ids = _ids_list(ids, "update_documents") if not update_vectors: return self.update_metadata( ids, @@ -621,7 +683,7 @@ def update_documents( if len(vectors) != len(ids): raise ValueError("ids and vectors must have equal length") - item_ids = _mutation_items(list(ids), "update_documents") + item_ids = _mutation_items(ids, "update_documents") packed = [pack_metadata(t, m) for t, m in zip(texts, metadatas)] return self._update_items( [ @@ -666,6 +728,8 @@ def upsert_documents( "inserted_item_ids": [], "not_found_item_ids": [], } + if ids is not None: + ids = _ids_list(ids, "upsert_documents") if ids is not None and len(ids) != len(documents): raise ValueError("ids and documents must have equal length") diff --git a/tests/test_vectorstore.py b/tests/test_vectorstore.py index 743a23f..01bacd5 100644 --- a/tests/test_vectorstore.py +++ b/tests/test_vectorstore.py @@ -1525,17 +1525,43 @@ def test_update_and_upsert_refuse_values_that_name_no_item(not_an_id): assert client.index.upserts == [] -@pytest.mark.parametrize("not_an_id", _COERCIBLE_NON_IDS) -def test_add_texts_inserts_instead_of_overwriting_on_a_non_id(not_an_id): - # add_texts keeps its existing rule for ids it cannot honour: insert as a - # new row and warn. It must not update item 3 in place. +@pytest.mark.parametrize("not_an_id", [True, 3.9, 3.0]) +def test_add_texts_raises_on_a_value_that_can_be_no_id(not_an_id): + # A float or bool is a wrongly-typed item ID, not a foreign ID: raising + # beats inserting a duplicate row with a warning. Item 3 is untouched. client = FakeClient() store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) store.add_texts(["a", "b", "c"]) + with pytest.raises(ValueError, match="is not an item ID"): + store.add_texts(["x"], ids=[not_an_id]) + assert client.index.upserts == [] + assert len(client.index.inserted) == 1 + assert client.index.stored[(None, 3)] == '{"text": "c", "metadata": {}}' + + +@pytest.mark.parametrize( + "foreign", + [ + "550e8400-e29b-41d4-a716-446655440000", + "doc-7", + "0", + "-3", + "+3", + "3.0", + "3_000", + "\u0663", + ], +) +def test_add_texts_inserts_a_string_that_is_not_an_item_id_with_a_warning(foreign): + # LangChain IDs are strings, so any string that is not one of ours is a + # foreign ID: a new row under a server-issued ID, plus a warning. This + # keeps 0-based chunk numbers working and never touches item 3. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) with pytest.warns(UserWarning, match="not enVector item IDs"): - new_ids = store.add_texts(["x"], ids=[not_an_id]) - assert new_ids == ["4"] + assert store.add_texts(["x"], ids=[foreign]) == ["4"] assert client.index.upserts == [] assert client.index.stored[(None, 3)] == '{"text": "c", "metadata": {}}' @@ -1588,3 +1614,95 @@ def test_numpy_integers_are_item_ids_and_numpy_bool_float_are_not(): store.delete([bad]) with pytest.raises(ValueError, match="positive integers"): store.delete([np.int64(0)]) + + +@pytest.mark.parametrize("bare", ["13", "1", b"13", bytearray(b"13")]) +def test_ids_must_be_a_list_not_a_bare_string(bare): + # A str is a sequence of its characters and bytes iterate as ints, so a + # bare "13" would address items 1 and 3, and b"13" items 49 and 51. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts([f"d{i}" for i in range(60)]) + doc = LC_Document(page_content="new") + + for call in ( + lambda: store.delete(bare), + lambda: store.get_by_ids(bare), + lambda: store.update_metadata(bare, ["t"]), + lambda: store.update_documents(bare, [doc]), + lambda: store.update_documents(bare, [doc], update_vectors=False), + lambda: store.upsert_documents([doc], ids=bare), + lambda: store.add_texts(["x"], ids=bare), + ): + with pytest.raises(TypeError, match=r"list of item IDs, e.g. \[doc.id\]"): + call() + assert client.index.deleted == [] and client.index.fetched == [] + assert client.index.updates == [] and client.index.upserts == [] + assert len(client.index.inserted) == 1 + + +def test_ids_accept_tuples_sets_and_generators(): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + assert sorted(d.page_content for d in store.get_by_ids(("1", "3"))) == ["a", "c"] + assert sorted(d.page_content for d in store.get_by_ids({"1", "3"})) == ["a", "c"] + assert [d.page_content for d in store.get_by_ids(x for x in ["2"])] == ["b"] + store.delete(("1", "3")) + assert sorted(client.index.deleted[-1]["item_ids"]) == [1, 3] + + +def test_digit_strings_longer_than_int64_are_not_ids(): + # CPython refuses int() on more than 4300 digits with its own ValueError; + # a 19-digit cap classifies such strings before int() ever runs. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a"]) + huge = "9" * 4301 + twenty = "1" + "0" * 19 + assert store.get_by_ids([huge, twenty]) == [] + assert client.index.fetched == [] + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete([huge]) + with pytest.warns(UserWarning, match="not enVector item IDs"): + assert store.add_texts(["x"], ids=[huge]) == ["2"] + + +@pytest.mark.parametrize("value", ["+3", "3_000", "3.0", "\u0663", "\uff13", b"3"]) +def test_numeric_looking_values_never_address_item_3(value): + # Accepted by int() on main (b"3" and "3_000" too), these name no item now. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + assert store.get_by_ids([value]) == [] + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete([value]) + assert client.index.fetched == [] and client.index.deleted == [] + + +def test_exotic_integer_like_objects_in_add_texts_raise(): + from decimal import Decimal + from fractions import Fraction + + np = pytest.importorskip("numpy") + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b", "c"]) + for value in (Decimal(3), Fraction(3), np.array(3), np.float64(3.0)): + with pytest.raises(ValueError, match="is not an item ID"): + store.add_texts(["x"], ids=[value]) + with pytest.raises(ValueError): + store.delete([value]) + assert client.index.upserts == [] and client.index.deleted == [] + + +def test_signed_zero_and_plus_three_messages(): + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a"]) + with pytest.raises(ValueError, match="positive integers"): + store.delete(["-0"]) + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete(["+0"]) + with pytest.raises(ValueError, match="expects integer item IDs"): + store.delete(["+3"]) From d5e4514a538dbcd9b78cc59a17b21fb2928f0ea2 Mon Sep 17 00:00:00 2001 From: Minseok Park Date: Tue, 6 Oct 2026 13:06:33 +0900 Subject: [PATCH 4/4] fix: treat ids=None as "nothing to update" again, as on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9fc0667 routed ids through _ids_list before the empty check, so update_metadata(None, ...) and update_documents(None, ...) raised TypeError where main returned an empty result, and disagreed with delete(None), which still did nothing. Skip the conversion for None in both. The None behaviour now matches LangChain's InMemoryVectorStore where it has the method — get_by_ids(None) raises TypeError, delete(None) does nothing, add_texts(ids=None) inserts — and, for update_* and upsert_documents which LangChain does not have, follows delete(None): nothing to update, or insert everything. A bare string is still refused. One test pins all six. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../langchain_envector/vectorstore.py | 6 ++-- tests/test_vectorstore.py | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/libs/envector/langchain_envector/vectorstore.py b/libs/envector/langchain_envector/vectorstore.py index ed46d03..a9011da 100644 --- a/libs/envector/langchain_envector/vectorstore.py +++ b/libs/envector/langchain_envector/vectorstore.py @@ -611,7 +611,8 @@ def update_metadata( matched no live row (missing or already deleted) — those are reported, not raised. """ - ids = _ids_list(ids, "update_metadata") + if ids is not None: # None, like an empty list, updates nothing + ids = _ids_list(ids, "update_metadata") if not ids: return {"request_id": [], "not_found_item_ids": []} if len(texts) != len(ids): @@ -656,7 +657,8 @@ def update_documents( texts = [getattr(d, "page_content", "") for d in documents] metadatas = [getattr(d, "metadata", {}) for d in documents] - ids = _ids_list(ids, "update_documents") + if ids is not None: # None, like an empty list, updates nothing + ids = _ids_list(ids, "update_documents") if not update_vectors: return self.update_metadata( ids, diff --git a/tests/test_vectorstore.py b/tests/test_vectorstore.py index 01bacd5..b454a74 100644 --- a/tests/test_vectorstore.py +++ b/tests/test_vectorstore.py @@ -1706,3 +1706,33 @@ def test_signed_zero_and_plus_three_messages(): store.delete(["+0"]) with pytest.raises(ValueError, match="expects integer item IDs"): store.delete(["+3"]) + + +def test_none_ids_behave_like_langchain(): + # LangChain's InMemoryVectorStore: get_by_ids(None) raises TypeError, + # delete(None) does nothing, add_texts(ids=None) inserts. update_* and + # upsert_documents have no LangChain counterpart; None updates nothing, + # like delete(None), and upsert_documents(ids=None) inserts. + client = FakeClient() + store = Envector(config=_cfg(), embeddings=FakeEmbeddings(dim=4), client=client) + store.add_texts(["a", "b"]) + doc = LC_Document(page_content="new") + + with pytest.raises(TypeError): + store.get_by_ids(None) + assert store.delete(None) is False + assert store.update_metadata(None, []) == { + "request_id": [], + "not_found_item_ids": [], + } + assert store.update_documents(None, []) == { + "request_id": [], + "not_found_item_ids": [], + } + assert store.update_documents(None, [], update_vectors=False) == { + "request_id": [], + "not_found_item_ids": [], + } + assert client.index.deleted == [] and client.index.updates == [] + assert store.add_texts(["c"], ids=None) == ["3"] + assert store.upsert_documents([doc], ids=None)["inserted_item_ids"] == [4]