fix: populate split_idx_start metadata in EmbeddingBasedDocumentSplitter#11987
Conversation
|
@rautaditya2606 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull request overview
Fixes inconsistency in splitter output metadata by adding split_idx_start to EmbeddingBasedDocumentSplitter chunks, aligning it with other splitter implementations and preventing downstream KeyErrors when consumers expect character offsets.
Changes:
- Populate
split_idx_startfor each emitted chunk by accumulating character offsets in_create_documents_from_splits(). - Document the new metadata field in
run()/run_async()docstrings. - Add unit regression tests covering both the internal chunk creation and the public
run()output, plus a release note.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
haystack/components/preprocessors/embedding_based_document_splitter.py |
Adds split_idx_start metadata and updates docstrings to describe the new field. |
test/components/preprocessors/test_embedding_based_document_splitter.py |
Adds regression tests asserting split_idx_start presence and correctness (including indexing back into the source text). |
releasenotes/notes/fix-embedding-splitter-missing-split-idx-start-8727a44789f5256a.yaml |
Release note documenting the bugfix and downstream impact. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
davidsbatista
left a comment
davidsbatista
left a comment
There was a problem hiding this comment.
Looks good! Thanks for the fix!
Related Issues
Fixes #11986
Proposed Changes
split_idx_startwhile creating output documents by tracking the accumulated character offset.run()andrun_async()docstrings to document the additional metadata field.test_create_documents_from_splits_split_idx_starttest_run_split_idx_startHow did you test it?
hatch run test:unit test/components/preprocessors/test_embedding_based_document_splitter.py -k "not integration"37 tests passed, 10 integration tests deselected.
Notes for the reviewer
This change makes
EmbeddingBasedDocumentSplitteremitsplit_idx_startconsistently with the other splitter implementations.Since the splitter constructs chunks through pure string concatenation without modifying or reordering text, the character offset is readily available by accumulating the lengths of the emitted chunks. The added regression tests verify both the metadata and that each reported offset correctly indexes into the original document.
Checklist