Skip to content

Fix numeric $in and $nin metadata filters - #139

Open
OsamaAnsar wants to merge 1 commit into
Stevenic:mainfrom
OsamaAnsar:fix/numeric-in-nin-filters
Open

OsamaAnsar wants to merge 1 commit into
Stevenic:mainfrom
OsamaAnsar:fix/numeric-in-nin-filters

Conversation

@OsamaAnsar

Copy link
Copy Markdown

What was wrong

In ItemSelector.metadataFilter, $in / $nin only ever compared a numeric metadata value against the string elements of the filter array, and only as a substring (val.includes(value)). Numbers in the filter array were never compared. The $nin branch also applied the substring test to string values. This looks like a leftover from the array-metadata fixes in #36 / #55. types.ts documents both operators as (number | string)[] and docs/api-reference.md says "Value in array".

Observed on current main (built lib/, Node 22):

FAIL number in numeric $in (expect true): got=false          { year: 2020 } vs $in [2019, 2020, 2021]
FAIL number in numeric $nin (expect false): got=true         { year: 2020 } vs $nin [2019, 2020, 2021]
FAIL 42 vs $in ["x42y"] (expect false): got=true             number matches as a substring of a string
FAIL string "b" vs $nin ["abc"] (expect true): got=false     string excluded by substring
FAIL array ["one","x"] vs $in ["one","two"] (expect true): got=false
e2e startPos $in [0,1]: matches = 0 (expected 5)             LocalIndex.queryItems, 5 items with startPos=0
e2e endPos $nin [20]: matches = 5 (expected 0)               5 items with endPos=20

After this change the same script prints ok for every case and e2e startPos $in [0,1]: matches = 5, e2e endPos $nin [20]: matches = 0.

The fix

$in / $nin now test exact membership (Array.prototype.includes, strict equality) via a small private helper isInList:

  • numbers are compared with numbers, strings with strings, no coercion (42 does not match '42' or 'x42y')
  • booleans still never match either operator (unchanged)
  • array-valued metadata (the categories: ['one'] case from $in filter not working as expected #35 / Fix $in filter #36 / Fix issue with $nin query similar to $in fix from PR #36 #55) keeps working and now works for any number of elements: $in matches if any element is in the filter array, $nin excludes if any element is. Previously this only worked for single-element arrays, by way of the array being coerced to a string by includes.
  • other operators are untouched

Tests: two existing tests were changed

Two existing characterization tests in src/ItemSelector.spec.ts asserted the old behavior and are replaced by this PR, because they directly encode the bug:

  • $in behavior matches implementation (... numbers via substring in array strings) asserted that { val: 42 } matches $in: ['x42y', 'baz'].
  • $nin behavior matches implementation (...) asserted that { val: 'oo' } is excluded by $nin: ['foobar', 'baz'], and (labelled "implementation detail") that { val: 42 } is kept by $nin: [42].

They are replaced by tests for exact string membership, numeric membership, "substring no longer matches", array-valued metadata, missing fields, and an end-to-end listItemsByMetadata / queryItems test in LocalIndex.spec.ts. If you would rather keep the old substring semantics for strings, I am happy to narrow this to numbers only, but substring matching for $nin looks unintentional to me.

Verification:

  • yarn lint: 0 errors (warnings in the three changed .ts files: 12 on main, 10 here)
  • yarn test: 598 passing
  • Revert check: with only src/ItemSelector.ts reverted to main, 5 of the new tests fail ($in compares numbers by value, $nin excludes only exact members ..., $nin compares numbers by value, $in and $nin match array-valued metadata ..., and supports numeric $in and $nin filters); with the fix restored all 598 pass.

Docs: docs/changelog.md (new Unreleased entry), llms.txt, and the operator table in docs/api-reference.md are updated.

CONTRIBUTING suggests opening an issue for bugs; I went straight to a PR with the repro above, but I'm happy to open an issue as well if you prefer.

$in and $nin only compared a numeric metadata value against the string
elements of the filter array, and only as a substring. As a result
{ year: { $in: [2019, 2020] } } matched nothing, $nin with numbers
excluded nothing, a number matched as a substring of a string element
(42 matched 'x42y'), and $nin excluded strings that were substrings of
an element ('b' was excluded by ['abc']).

Both operators now test exact membership with strict equality. Booleans
still never match, and array-valued metadata matches when any element
is in the filter array ($in) or excluded when any element is ($nin).

The two existing $in/$nin tests asserted the old substring behavior
("matches implementation"), so they are replaced with tests for the
documented semantics.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant