Skip to content

Commit b159da4

Browse files
fix: keep insertion order when MetaFieldGroupingRanker sort values have mixed types (#11942)
Co-authored-by: bogdankostic <bogdankostic@web.de>
1 parent 883c508 commit b159da4

3 files changed

Lines changed: 42 additions & 4 deletions

File tree

haystack/components/rankers/meta_field_grouping_ranker.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,11 @@
44

55
from collections import defaultdict
66

7-
from haystack import Document, component
7+
from haystack import Document, component, logging
88
from haystack.utils.misc import _deduplicate_documents
99

10+
logger = logging.getLogger(__name__)
11+
1012

1113
@component
1214
class MetaFieldGroupingRanker:
@@ -117,9 +119,21 @@ def run(self, documents: list[Document]) -> dict[str, list[Document]]:
117119
for docs in subgroups.values():
118120
if sort_field:
119121
# Sort by the field value, placing documents with a missing value last.
120-
# The (is_missing, value) tuple ensures that only actual field values are
121-
# compared, making the sort work for numbers, strings, and other types.
122-
docs.sort(key=lambda d: (d.meta.get(sort_field) is None, d.meta.get(sort_field)))
122+
# The (is_missing, value) tuple keeps documents with a missing value out of the
123+
# value comparison, but two present values of mutually non-comparable types
124+
# (e.g. an int and a str) would still raise a TypeError. In that case we keep the
125+
# group's insertion order instead of crashing, mirroring MetaFieldRanker.
126+
try:
127+
docs.sort(key=lambda d: (d.meta.get(sort_field) is None, d.meta.get(sort_field)))
128+
except TypeError as error:
129+
logger.warning(
130+
"Tried to sort Documents with IDs {document_ids}, but got TypeError with the "
131+
"message: {error}\nKeeping the original order of the Documents in this group "
132+
"since sorting by '{sort_field}' is not possible.",
133+
document_ids=",".join([doc.id for doc in docs]),
134+
error=error,
135+
sort_field=sort_field,
136+
)
123137
ordered_docs.extend(docs)
124138

125139
ordered_docs.extend(no_group_docs)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
fixes:
3+
- |
4+
Fixed ``MetaFieldGroupingRanker`` crashing with an uncaught ``TypeError`` when a group contained
5+
``sort_docs_by`` values of mutually non-comparable types (for example an ``int`` and a ``str``). The
6+
sort is now wrapped in a ``try``/``except``, so in that case the group's original insertion order is
7+
kept and a warning is logged, mirroring the behavior of ``MetaFieldRanker``.

test/components/rankers/test_meta_field_grouping_ranker.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,23 @@ def test_run_metadata_with_different_data_types(self) -> None:
185185
assert result["documents"][1].meta["group"] == 42
186186
assert result["documents"][2].meta["group"] is True
187187

188+
def test_run_sort_docs_by_mixed_uncomparable_types(self) -> None:
189+
"""
190+
Test that the ranker does not crash when a group's sort_docs_by values have mutually
191+
non-comparable present types (e.g. int and str), keeping the group's insertion order instead.
192+
"""
193+
docs_with_mixed_sort_values = [
194+
Document(content="int value", meta={"group": "g1", "split_id": 3}),
195+
Document(content="str value", meta={"group": "g1", "split_id": "10"}),
196+
]
197+
sample_ranker = MetaFieldGroupingRanker(group_by="group", sort_docs_by="split_id")
198+
result = sample_ranker.run(documents=docs_with_mixed_sort_values)
199+
assert "documents" in result
200+
assert len(result["documents"]) == 2
201+
# Insertion order is preserved because the values cannot be compared.
202+
assert result["documents"][0].content == "int value"
203+
assert result["documents"][1].content == "str value"
204+
188205
def test_run_deduplicates_documents(self) -> None:
189206
"""
190207
Test that duplicate documents are removed before grouping.

0 commit comments

Comments
 (0)