fix: sort SentenceWindowRetriever context before merging text - #12976
Merged
bogdankostic merged 7 commits intoOct 2, 2026
Merged
bogdankostic merged 7 commits into
bogdankostic merged 7 commits into
Conversation
The context documents were merged into `context_windows` before being sorted by `split_id`. When chunks have no `split_idx_start` (for example chunks from MarkdownHeaderSplitter), `merge_documents_text` concatenates them in the order returned by the Document Store, so the context text could be scrambled while `context_documents` was correctly sorted. Sort first and merge the sorted list, in both run and run_async. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
LindseyZ1205
requested review from
bogdankostic
and removed request for
a team
September 26, 2026 20:18
Contributor
|
@LindseyZ1205 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
6 of 7 tasks
Contributor
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
The run/run_async docstrings said context_documents are sorted by split_idx_start. They are sorted by split_id_meta_field within each window, grouped in the order of retrieved_documents, and documents shared by overlapping windows appear once per window. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The Output variables row said context_documents are ordered by split_idx_start. They are grouped by retrieved document and ordered by split_id within each window. Updated docs/ and the current stable version-3.3 page. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Document the metadata the component relies on (source_id, split_id, optional split_idx_start), which splitters produce it, what happens when it is missing, and what the two outputs contain. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Bring back the technique name and the indexing/retrieval workflow that the previous overview rewrite dropped. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
bogdankostic
approved these changes
Oct 2, 2026
bogdankostic
left a comment
bogdankostic
left a comment
Contributor
There was a problem hiding this comment.
Thanks for this PR and congrats on your first contribution to Haystack! 🎉
As you suggested in the PR description, I fixed the run/run_async docstrings and updated the documentation page.
This branch was successfully deployed
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
2 participants
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.
Related Issues
SentenceWindowRetrieverreturnscontext_windowsin Document Store order when chunks have nosplit_idx_start#12975Proposed Changes:
The shared
_assemble_contextpath merges context text before sorting documents bysplit_id. When chunks have nosplit_idx_start(for example chunks fromMarkdownHeaderSplitter),merge_documents_textconcatenates them in the Document Store's return order, whilecontext_documentsis sorted. Sort once before assembling both outputs. This preserves the upstream batched retrieval used by bothrunandrun_async.How did you test it?
hatch run fmt-checkandhatch run test:typespass for the changed source and two test files;git diff --checkpasses.Notes for the reviewer
The
run()docstring sayscontext_documentsare sorted by thesplit_idx_startmeta field, but the code sorts bysplit_id_meta_field. I left it unchanged to keep this PR focused, happy to update it here if you prefer.AI assistance: Claude Code assisted with the original bug investigation and patch. Codex assisted with adapting the fix to upstream batched retrieval, resolving the merge conflict, and running the checks above.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.🤖 Generated with Claude Code