fix: page_number for form feeds inside a RecursiveDocumentSplitter chunk - #12590
fix: page_number for form feeds inside a RecursiveDocumentSplitter chunk#12590dylanpulver wants to merge 1 commit into
Conversation
The splitter counted every form feed in a chunk before assigning that chunk's page_number, so a chunk was labelled with the page it ends on rather than the page its text starts on. The existing correction only applied when the form feed was the chunk's last character, which any separator other than "\f" prevents. page_number is now derived from the chunk's start offset in the source document, which the component already tracks for split_idx_start.
|
@dylanpulver is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Hi @dylanpulver, 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 |
Related Issues
None — found comparing how the splitters compute
page_number.Proposed Changes:
RecursiveDocumentSplittercounted every\fin a chunk before assigning itspage_number, so a chunk got the page it ends on, not the page its text starts on.A correction exists, but it only strips form feeds that are the chunk's very last characters (
chunk.rstrip("\f")). With any separator other than"\f"the break is followed by the separator, so it never fires —"another page. \f "ends with a space.page_numbernow comes from the chunk's start offset, already tracked forsplit_idx_start. Leading form feeds still advance the chunk's page, so that is unchanged."aa bb\fcc dd ee ff\fgg hh",split_length=4,separators=[" "],split_unit="word":"aa bb\fcc dd ee ""ff\fgg hh"DocumentSplitterreturns 1 and 2 for this text.How did you test it?
New unit test for a break inside a chunk.
hatch run test:unit test: 6360 passed before, 6361 after, no failures either side.fmtandtest:typesclean on the changed files.Two existing assertions changed from 3 to 2, both on the chunk starting
"another page."— that text is on page 2. The other four page assertions in each test are unchanged.Notes for the reviewer
This PR was generated with an AI assistant (Claude Code, claude-opus-5). I reviewed the changes and ran the relevant tests.