Skip to content

fix: keep insertion order when MetaFieldGroupingRanker sort values have mixed types - #11942

Merged
bogdankostic merged 2 commits into
deepset-ai:mainfrom
otiscuilei:fix/meta-field-grouping-ranker-mixed-type-sort
Jul 15, 2026
Merged

fix: keep insertion order when MetaFieldGroupingRanker sort values have mixed types#11942
bogdankostic merged 2 commits into
deepset-ai:mainfrom
otiscuilei:fix/meta-field-grouping-ranker-mixed-type-sort

Conversation

@otiscuilei

Copy link
Copy Markdown
Contributor

MetaFieldGroupingRanker sorts the documents inside each group by a metadata field when sort_docs_by is set. The sort key (value is None, value) correctly pushes documents with a missing sort value to the end, but it does not protect against two documents that both have a present value of mutually non-comparable types. When a group contains, for example, one split_id that is an int and another that is a str, both keys evaluate to (False, <value>) and Python compares the raw values, raising TypeError: '<' not supported between instances of 'str' and 'int'. Because the sort was not guarded, the exception propagates out of run() and aborts the whole component/pipeline.

Heterogeneous metadata types for a field like split_id, page, or date are realistic in RAG setups where documents come from different converters or document stores (for instance JSON-sourced strings mixed with int-typed values). Any user running such documents through this ranker with sort_docs_by set hits the crash.

This is also inconsistent with the sibling MetaFieldRanker, which already wraps its sorted(...) in a try/except TypeError and falls back to the original order with a warning. This change mirrors that behavior: the per-group sort is wrapped in try/except TypeError, and on failure the group's original insertion order is kept and a warning is logged. The misleading comment claiming the tuple makes the sort work "for numbers, strings, and other types" was corrected.

Before:

from haystack import Document
from haystack.components.rankers import MetaFieldGroupingRanker

docs = [
    Document(content="a", meta={"group": "g1", "split_id": 3}),
    Document(content="b", meta={"group": "g1", "split_id": "10"}),
]
ranker = MetaFieldGroupingRanker(group_by="group", sort_docs_by="split_id")
ranker.run(documents=docs)
# TypeError: '<' not supported between instances of 'str' and 'int'

After:

result = ranker.run(documents=docs)
# No exception; a warning is logged and the group keeps insertion order:
# [Document(content="a", ...), Document(content="b", ...)]

A release note is included at releasenotes/notes/meta-field-grouping-ranker-mixed-type-sort-a1b2c3d4e5f60718.yaml.

@otiscuilei
otiscuilei requested a review from a team as a code owner July 10, 2026 05:50
@otiscuilei
otiscuilei requested review from bogdankostic and removed request for a team July 10, 2026 05:50
@vercel

vercel Bot commented Jul 10, 2026

Copy link
Copy Markdown

@Otis0408 is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot added topic:tests type:documentation Improvements on the docs labels Jul 10, 2026
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  haystack/components/rankers
  meta_field_grouping_ranker.py
Project Total  

This report was generated by python-coverage-comment-action

@otiscuilei
otiscuilei force-pushed the fix/meta-field-grouping-ranker-mixed-type-sort branch from a2feaa5 to b7a3a86 Compare July 10, 2026 06:01

@bogdankostic bogdankostic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks @Otis0408!

@bogdankostic
bogdankostic merged commit b159da4 into deepset-ai:main Jul 15, 2026
22 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants