Skip to content

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

Open
Otis0408 wants to merge 1 commit into
deepset-ai:mainfrom
Otis0408:fix/meta-field-grouping-ranker-mixed-type-sort
Open

fix: keep insertion order when MetaFieldGroupingRanker sort values have mixed types#11942
Otis0408 wants to merge 1 commit into
deepset-ai:mainfrom
Otis0408:fix/meta-field-grouping-ranker-mixed-type-sort

Conversation

@Otis0408

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.

@Otis0408 Otis0408 requested a review from a team as a code owner July 10, 2026 05:50
@Otis0408 Otis0408 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

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

@Otis0408 Otis0408 force-pushed the fix/meta-field-grouping-ranker-mixed-type-sort branch from a2feaa5 to b7a3a86 Compare July 10, 2026 06:01
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.

1 participant