fix: deep-copy document metadata in Markdown, CSV and hierarchical splitters - #12249
fix: deep-copy document metadata in Markdown, CSV and hierarchical splitters#12249bharadwaj-pendyala wants to merge 3 commits into
Conversation
…litters MarkdownHeaderSplitter and CSVDocumentSplitter copied the input document's meta shallowly, so nested values were shared between every chunk they emitted and with the caller's document. Editing one chunk's metadata changed all the others. HierarchicalDocumentSplitter had the same problem on its root node. DocumentSplitter, RecursiveDocumentSplitter and EmbeddingBasedDocumentSplitter already deep-copy, so this brings the three outliers in line.
|
@bharadwaj-pendyala is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
See #12248 (comment) |
|
Hi @bharadwaj-pendyala, thanks a lot for your contribution! 🙏 We noticed that the Contributor License Agreement (CLA) check ( To get your PR reviewed, please sign the CLA via the link in the |
|
Thanks for signing the CLA, @bharadwaj-pendyala! 🎉 This PR is now ready for review again and the reviewer has been re-assigned. |
| --- | ||
| fixes: | ||
| - | | ||
| `MarkdownHeaderSplitter` and `CSVDocumentSplitter` now deep-copy the metadata of the document they split, |
There was a problem hiding this comment.
We use restructured text markdown so for inline comments please use double back ticks
| `MarkdownHeaderSplitter` and `CSVDocumentSplitter` now deep-copy the metadata of the document they split, | |
| ``MarkdownHeaderSplitter`` and ``CSVDocumentSplitter`` now deep-copy the metadata of the document they split, |
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||
|
recheck |
Related Issues
Proposed Changes:
MarkdownHeaderSplitterandCSVDocumentSplittercopied the input document'smetashallowly, so every chunk they emitted held the same nested objects as its siblings and as the caller's document.HierarchicalDocumentSplitterhad the same problem on its root node.On
main(51f78e5) that prints['docs', 'install'] ['docs', 'install']. Both the sibling chunk and the caller's document picked up an edit made to the first chunk.The fix is
deepcopy, matchingDocumentSplitter(document_splitter.py:325),RecursiveDocumentSplitter(recursive_splitter.py:437),EmbeddingBasedDocumentSplitter(embedding_based_document_splitter.py:501),DocumentCleanerandCSVDocumentCleaner. Those five were already correct; these three were the outliers.For
HierarchicalDocumentSplitterthe copy goes at the root call site rather than inside_add_meta_data._add_meta_dataalso runs on every child, and children come out ofDocumentSplitterwith metadata that has already been copied, so putting it there would copy each child's metadata a second time for nothing.Scope
Only the three splitters. The extractors at
llm_document_content_extractor.py:354andllm_metadata_extractor.py:364also build{**document.meta}, but they emit one document per input rather than many chunks, so I left them alone.I did not change
Documentto copymetaon construction. That would put the cost on every document in every pipeline and change identity semantics repo-wide, which is a bigger call than this bug needs. The convention here is that the component producing new documents owns the copy, same as #12107 did fornormalize_metadata.How did you test it?
Three regression tests, one per component. All three fail on
mainand pass with the fix:Full gates on Python 3.14, macOS:
hatch run test:unit test/components/preprocessors/: 354 passed, 19 deselectedhatch run test:unit: 5930 passed, 8 skipped, 2 failed. Both failures are intest/utils/test_type_serialization.py(test_output_type_serialization_typing_generic_with_nonetype,test_output_type_serialization_callable_with_parameter_list) and reproduce on a cleanmaincheckout, so they are not from this change.hatch run test:types: Success, no issues found in 391 source fileshatch run fmt-check: All checks passedNotes for the reviewer
Cost, since
deepcopyis not free andCSVDocumentSplitternow does it once per sub-table. Measured on this branch, best of five runs:{"file_path", "source_id", "tags": [2 items]}Realistic metadata is noise. The second row is a deliberately unpleasant payload and still only costs 37ms across 40 chunks. Copying per chunk is what makes siblings independent, so hoisting a single copy out of the loop would fix the leak into the caller's document but leave the chunks sharing state with each other.
Document.metais documented as "Must be JSON-serializable" (document.py:42), sodeepcopyis safe for anything the contract allows, and splitDocument.ids are unchanged because ids come fromjson.dumps(self.meta, sort_keys=True, ...).This PR was fully generated with an AI assistant. I have reviewed the changes and run the tests and checks listed above.
Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.