From 407739e1d687957efcc067631c4e30362a56895d Mon Sep 17 00:00:00 2001 From: Timur Rakhmatullin Date: Mon, 21 Sep 2026 22:11:04 -0700 Subject: [PATCH] fix: count page breaks in overlapping regions once in RecursiveDocumentSplitter With split_overlap > 0 the overlapping tail of a chunk is repeated at the start of the next chunk, so a page break character inside that region was counted once per chunk containing it. Every chunk after the break then carried a page_number shifted upwards by one per repetition, which can exceed the number of pages actually present in the document. Advance the page counter only over the characters the next chunk does not repeat, mirroring how current_position is already advanced. Behavior with split_overlap=0 is unchanged. --- .../preprocessors/recursive_splitter.py | 10 +++++-- ...r-overlap-page-count-44ea892ea232aabc.yaml | 7 +++++ .../preprocessors/test_recursive_splitter.py | 30 +++++++++++++++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/fix-recursive-splitter-overlap-page-count-44ea892ea232aabc.yaml diff --git a/haystack/components/preprocessors/recursive_splitter.py b/haystack/components/preprocessors/recursive_splitter.py index 168a0437151..6fd7d18c76c 100644 --- a/haystack/components/preprocessors/recursive_splitter.py +++ b/haystack/components/preprocessors/recursive_splitter.py @@ -464,16 +464,16 @@ def _run_one(self, doc: Document) -> list[Document]: self._add_overlap_info(current_position, new_doc, new_docs) # count page breaks in the chunk - current_page += chunk.count("\f") + chunk_page = current_page + chunk.count("\f") # if there are consecutive page breaks at the end with no more text, adjust the page number # e.g: "text\f\f\f" -> 3 page breaks, but current_page should be 1 consecutive_page_breaks = len(chunk) - len(chunk.rstrip("\f")) if consecutive_page_breaks > 0: - new_doc.meta["page_number"] = current_page - consecutive_page_breaks + new_doc.meta["page_number"] = chunk_page - consecutive_page_breaks else: - new_doc.meta["page_number"] = current_page + new_doc.meta["page_number"] = chunk_page # keep the new chunk doc and update the current position new_docs.append(new_doc) @@ -485,6 +485,10 @@ def _run_one(self, doc: Document) -> list[Document]: overlap_char_len = len(overlap_str) else: overlap_char_len = 0 + # Advance the page counter only over the characters the next chunk does not repeat: the + # overlapping tail reappears at the start of the next chunk, so a page break inside it would + # otherwise be counted once per chunk and shift every following page number upwards. + current_page += chunk.count("\f", 0, len(chunk) - overlap_char_len) current_position += len(chunk) - overlap_char_len return new_docs diff --git a/releasenotes/notes/fix-recursive-splitter-overlap-page-count-44ea892ea232aabc.yaml b/releasenotes/notes/fix-recursive-splitter-overlap-page-count-44ea892ea232aabc.yaml new file mode 100644 index 00000000000..20df081fd05 --- /dev/null +++ b/releasenotes/notes/fix-recursive-splitter-overlap-page-count-44ea892ea232aabc.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed ``RecursiveDocumentSplitter`` counting a page break character (``\f``) once per chunk it appears in when + ``split_overlap`` is used. A page break inside an overlapping region is repeated in consecutive chunks, so the + ``page_number`` metadata of every following chunk was shifted upwards and could exceed the number of pages in the + document. The page counter now advances only over the part of a chunk that the next chunk does not repeat. diff --git a/test/components/preprocessors/test_recursive_splitter.py b/test/components/preprocessors/test_recursive_splitter.py index cf6dbbf0d44..b9266b2d416 100644 --- a/test/components/preprocessors/test_recursive_splitter.py +++ b/test/components/preprocessors/test_recursive_splitter.py @@ -365,6 +365,36 @@ def test_run_split_by_sentence_count_page_breaks_split_unit_char() -> None: assert chunks_docs[6].meta["split_idx_start"] == text.index(chunks_docs[6].content) +def test_run_count_page_breaks_once_with_overlap_split_unit_char() -> None: + # A page break that falls inside an overlapping region is repeated in consecutive chunks. It must only + # advance the page counter once, otherwise every chunk after it gets a page number that is too high. + splitter = RecursiveDocumentSplitter(split_length=14, split_overlap=5, separators=[" "], split_unit="char") + + text = "This is page one.\fThis is page two, it is longer." + + documents = splitter.run(documents=[Document(content=text)]) + chunks_docs = documents["documents"] + assert len(chunks_docs) == 5 + + assert chunks_docs[0].content == "This is page " + assert chunks_docs[0].meta["page_number"] == 1 + + # the page break is part of this chunk and of the overlapping tail repeated in the next chunk + assert chunks_docs[1].content == "page one.\fThis" + assert chunks_docs[1].meta["page_number"] == 2 + + # the repeated page break was previously counted a second time, pushing these chunks to page 3 + # even though the document only has two pages + assert chunks_docs[2].content == "\fThis is page " + assert chunks_docs[2].meta["page_number"] == 2 + + assert chunks_docs[3].content == "page two, it i" + assert chunks_docs[3].meta["page_number"] == 2 + + assert chunks_docs[4].content == " it is longer." + assert chunks_docs[4].meta["page_number"] == 2 + + def test_run_split_document_with_overlap_character_unit(): splitter = RecursiveDocumentSplitter(split_length=20, split_overlap=10, separators=["."], split_unit="char") text = """A simple sentence1. A bright sentence2. A clever sentence3"""