fix: validate is_in/not_in receive a list in the filter builder#683
Open
NishchayMahor wants to merge 1 commit into
Open
fix: validate is_in/not_in receive a list in the filter builder#683NishchayMahor wants to merge 1 commit into
NishchayMahor wants to merge 1 commit into
Conversation
Field.is_in() and Field.not_in() accepted any value, so Field("genre").is_in("drama")
silently produced {"genre": {"$in": "drama"}} — an invalid filter the server
rejects with an opaque 400. This is inconsistent with gt/gte/lt/lte, which already
validate via _require_numeric. Add a matching _require_list guard (accepts list or
tuple) and call it from is_in/not_in, with unit tests mirroring the numeric ones.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
In the metadata filter builder,
Field.is_in()/Field.not_in()accept any value without checking it's a list. A common mistake — passing a bare value instead of a list — is silently turned into an invalid filter:Because a string is iterable, nothing catches this client-side; the server rejects it later with an opaque
400 Invalid request, which is hard to trace back to the filter.This is inconsistent with the numeric operators in the same class —
gt/gte/lt/ltealready validate their argument via_require_numericand raise a clearTypeError.Fix
Add a
_require_listguard mirroring the existing_require_numeric, and call it fromis_in/not_in(pinecone/utils/filter_builder.py). It accepts alistortupleand raises a clearTypeErrorotherwise:Valid
list/tupleusage is unchanged.Testing
Added tests to
TestFieldSetOperatorsintests/unit/utils/test_filter_builder.py, mirroring the existingtest_numeric_only_*cases (tuple accepted;is_in/not_inwith a non-list raiseTypeErrormatching "list"). Verified the behavior directly against the module —filter_builder.pyis pure Python (list/tuple pass,str/intraise).Note
Low Risk
Small, localized change to filter-builder validation with no auth, data, or API contract changes beyond catching invalid client usage earlier.
Overview
Field.is_in()andField.not_in()now validate their argument client-side via a new_require_listhelper (same pattern as_require_numericon comparison operators).Passing a bare scalar (e.g.
Field("genre").is_in("drama")) no longer builds an invalid{"$in": "drama"}filter; it raisesTypeError: is_in requires a list of values, got str.listandtupleinputs behave as before.Unit tests cover tuple acceptance and
TypeErrorfor non-list values on both set operators.Reviewed by Cursor Bugbot for commit 47a999d. Bugbot is set up for automated code reviews on this repo. Configure here.