fix: recursive_splitter validation message and docstring accuracy - #12440
Merged
sjrl merged 3 commits intoAug 24, 2026
Conversation
- recursive_splitter only rejects negative split_overlap (0 is the default), but the error said 'must be greater than zero'; document_splitter/hierarchical_document_splitter already word this check as 'must be greater than or equal to 0' - recursive_splitter split_overlap is measured in split_units (word/char/token), not always characters - answer_joiner sort_by_score docstring said 'documents' (copy-paste from DocumentJoiner); it sorts answers - answer_joiner _concatenate docstring claimed it sorts by score; it only flattens (sorting happens in run() and only when sort_by_score) - meta_field ranker run() docstring referenced a nonexistent 'score' mode; valid values are reciprocal_rank_fusion and linear_score
simpleqt
requested review from
julian-risch
and
a lite review from Copilot
and removed request for
a team
August 23, 2026 13:00
|
@simpleqt is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
sjrl
added
the
ignore-for-release-notes
sjrl
reviewed
Aug 24, 2026
The CI format check flagged one over-long docstring line, and the reno check requires a release notes file for every PR.
sjrl
reviewed
Aug 24, 2026
sjrl
removed
the
ignore-for-release-notes
Contributor
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
All verified at HEAD:
recursive_splitter: the validation only rejects negativesplit_overlap(0 is the default), but the error message said "Overlap must be greater than zero-️ —document_splitterandhierarchical_document_splitteralready word this identical check as "must be greater than or equal to 0". Also,split_overlapis measured insplit_units(word/char/token), not always characters —_get_overlap()slices by unit and_run_one()even carries the comment 'split_overlap is in split_units, not chars'.answer_joiner:sort_by_scoredocstring said 'documents' (verbatim copy-paste fromDocumentJoiner); it sorts answers._concatenatedocstring claimed it "sorts by score-️ but it only flattens (sorting happens inrun()and only whensort_by_score=True).meta_fieldranker:run()docstring referenced a nonexistent'score'mode; valid values arereciprocal_rank_fusionandlinear_score(the__init__docstring for the same parameter already sayslinear_score).