fix(local): keep facet bool and int values distinct (#1389)

* fix(local): keep facet bool and int values distinct

* fix: refactor facet type fix, update tests

---------

Co-authored-by: George Panchuk <george.panchuk@qdrant.tech>

qdrant_client/local/local_collection.py

@@ -1262,7 +1262,14 @@ class LocalCollection:
         facet_filter: types.Filter | None = None,
         limit: int = 10,
     ) -> types.FacetResponse:
-        facet_hits: dict[types.FacetValue, int] = defaultdict(int)
+        # (bool, int, str). A value's position in this tuple is used as a type tag, so
+        # that `False`/`0` and `True`/`1` (equal and same-hash in python) don't collapse
+        # into a single bucket, and so that values of different types stay totally
+        # ordered when their counts tie. The server never mixes types in one facet (it
+        # reads a single payload index), so this cross-type order is local-mode only.
+        value_types = get_args_subscribed(types.FacetValue)
+
+        facet_hits: dict[tuple[int, types.FacetValue], int] = defaultdict(int)
 
         mask = self._payload_and_non_deleted_mask(facet_filter)
 
@@ -1279,11 +1286,11 @@ class LocalCollection:
                 continue
 
             # Only count the same value for each point once
-            values_set: set[types.FacetValue] = set()
+            values_set: set[tuple[int, types.FacetValue]] = set()
 
             # Sanitize to use only valid values
             for v in values:
-                if type(v) not in get_args_subscribed(types.FacetValue):
+                if type(v) not in value_types:
                     continue
 
                 # If values are UUIDs, format with hyphens
@@ -1291,14 +1298,14 @@ class LocalCollection:
                 if as_uuid:
                     v = str(as_uuid)
 
-                values_set.add(v)
+                values_set.add((value_types.index(type(v)), v))
 
             for v in values_set:
                 facet_hits[v] += 1
 
         hits = [
             models.FacetValueHit(value=value, count=count)
-            for value, count in sorted(
+            for (_, value), count in sorted(
                 facet_hits.items(),
                 # order by count descending, then by value ascending
                 key=lambda x: (-x[1], x[0]),

qdrant_client/local/tests/test_facet.py

@@ -0,0 +1,74 @@
+from qdrant_client.http import models
+from qdrant_client.local.qdrant_local import QdrantLocal
+
+COLLECTION_NAME = "test_facet"
+
+
+def facet_of(points: list[models.PointStruct]) -> list[tuple[type, models.FacetValue, int]]:
+    client = QdrantLocal(":memory:")
+    client.create_collection(COLLECTION_NAME, vectors_config={})
+    client.upsert(COLLECTION_NAME, points=points)
+
+    hits = client.facet(COLLECTION_NAME, key="a").hits
+
+    return [(type(hit.value), hit.value, hit.count) for hit in hits]
+
+
+def test_facet_keeps_scalar_types_distinct():
+    """`False == 0` and `True == 1` in python, but they are distinct facet values.
+
+    A facet on the server reads a single payload index, so a bool index only ever
+    returns bools and an integer index only ever returns ints. Local mode facets the
+    raw payload, so it has to keep the types apart on its own.
+    """
+    # every bucket ties at one point, so this also pins the tie-break order
+    assert facet_of(
+        [
+            models.PointStruct(id=1, vector={}, payload={"a": [False, 0]}),
+            models.PointStruct(id=2, vector={}, payload={"a": True}),
+            models.PointStruct(id=3, vector={}, payload={"a": 1}),
+            models.PointStruct(id=4, vector={}, payload={"a": "0"}),
+        ]
+    ) == [
+        (bool, False, 1),
+        (bool, True, 1),
+        (int, 0, 1),
+        (int, 1, 1),
+        (str, "0", 1),
+    ]
+
+
+def test_facet_does_not_compare_values_of_different_types():
+    """Ties on count fall through to the value, and `"a" < 1` raises in python."""
+    assert facet_of(
+        [
+            models.PointStruct(id=1, vector={}, payload={"a": 1}),
+            models.PointStruct(id=2, vector={}, payload={"a": "a"}),
+        ]
+    ) == [
+        (int, 1, 1),
+        (str, "a", 1),
+    ]
+
+
+def test_facet_normalizes_uuids_before_deduplicating():
+    """Both spellings of one uuid are a single value, counted once for the point.
+
+    This matches a uuid index on the server. Local mode has no index awareness, so it
+    normalizes unconditionally; a keyword index on the server would instead keep the
+    two spellings as two separate values.
+    """
+    assert facet_of(
+        [
+            models.PointStruct(
+                id=1,
+                vector={},
+                payload={
+                    "a": [
+                        "550e8400e29b41d4a716446655440000",
+                        "550e8400-e29b-41d4-a716-446655440000",
+                    ]
+                },
+            ),
+        ]
+    ) == [(str, "550e8400-e29b-41d4-a716-446655440000", 1)]

tests/congruence_tests/test_facet.py

@@ -206,3 +206,58 @@ def test_other_types_in_local():
 
     # Assertion is that it doesn't raise an exception
     client.facet(collection_name=collection_name, key="a")
+
+
+# `False == 0` and `True == 1` in python, so a key holding both bools and ints is where
+# local mode risks merging them into a single bucket. Every key below carries the same
+# mixed values, but gets a different payload index on the server: (key, the index to
+# build on it, the type of value that faceting through that index yields).
+MIXED_TYPE_INDEXES = [
+    ("mixed_bool", models.PayloadSchemaType.BOOL, bool),
+    ("mixed_int", models.PayloadSchemaType.INTEGER, int),
+    ("mixed_str", models.PayloadSchemaType.KEYWORD, str),
+]
+MIXED_TYPE_VALUES = [[False, 0], True, 1, "0", [True, 1], [True, True]]
+
+
+def test_mixed_scalar_types():
+    """Facet a key whose values span bools, ints and strings.
+
+    A facet on the server reads a single payload index, so it only ever returns values
+    of that index's type. Local mode facets the raw payload and returns every type at
+    once, so its hits are compared per type against the matching indexed key.
+    """
+    collection_name = f"{COLLECTION_NAME}_mixed_facet"
+    points = [
+        models.PointStruct(
+            id=idx,
+            vector=[0.1, 0.2],
+            payload={key: values for key, _, _ in MIXED_TYPE_INDEXES},
+        )
+        for idx, values in enumerate(MIXED_TYPE_VALUES)
+    ]
+    vectors_config = models.VectorParams(size=2, distance=models.Distance.DOT)
+
+    local_client = init_local()
+    init_client(local_client, points, collection_name, vectors_config=vectors_config)
+
+    remote_client = init_remote()
+    init_client(remote_client, points, collection_name, vectors_config=vectors_config)
+    for key, schema, _ in MIXED_TYPE_INDEXES:
+        remote_client.create_payload_index(collection_name, key, field_schema=schema)
+
+    def hits(client: QdrantBase, facet_key: str) -> list[models.FacetValueHit]:
+        return client.facet(
+            collection_name=collection_name, key=facet_key, limit=100, exact=True
+        ).hits
+
+    for key, _, value_type in MIXED_TYPE_INDEXES:
+        remote_hits = hits(remote_client, key)
+        local_hits = [hit for hit in hits(local_client, key) if type(hit.value) is value_type]
+
+        # compare the types explicitly: pydantic considers FacetValueHit(value=True)
+        # and FacetValueHit(value=1) equal, which is the very thing under test here
+        assert [(type(hit.value), hit.value, hit.count) for hit in local_hits] == [
+            (type(hit.value), hit.value, hit.count) for hit in remote_hits
+        ]
+        assert remote_hits, f"no {value_type.__name__} values were faceted for {key}"