From 81f06302f2bd9e9bf1940dbbeb62bd65d6c9db90 Mon Sep 17 00:00:00 2001 From: chrikrah <48338417+chrikrah@users.noreply.github.com> Date: Sat, 26 Sep 2026 13:46:28 +0000 Subject: [PATCH] fix(chunking): reject a negative overlap instead of corrupting text ChunkingOptions._validate() checked new_after_n_chars and new_after_n_tokens for negative values but never checked overlap. A negative overlap then corrupted output two ways with no error and no warning. On the character-split path, base.py:2731 computes the remainder as s[maxlen - overlap:], so a negative overlap starts the remainder past the end of the fragment and drops that many characters at every split boundary. A 60-character input at max_characters=20 and overlap=-5 loses 'UVWXY' and 'tuvwx' from the output entirely. With overlap_all=True, base.py:755 computes self._text[-overlap:], which for a negative overlap is a forward slice, so the tail carried into the next chunk repeats most of the current one. Reachable from chunk_elements, chunk_by_title and partition(chunking_strategy=). Now raises ValueError, matching the two sibling options. --- CHANGELOG.md | 6 ++++++ test_unstructured/chunking/test_base.py | 16 ++++++++++++++++ unstructured/__version__.py | 2 +- unstructured/chunking/base.py | 9 +++++++++ 4 files changed, 32 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ad71b4196f..28a223da70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.27.10 + +### Fixes + +- **Reject a negative `overlap`.** `ChunkingOptions` validated `new_after_n_chars` and `new_after_n_tokens` but not `overlap`, so a negative value silently corrupted output: on the character-split path the remainder started past the end of the fragment and dropped that many characters at every split boundary, and with `overlap_all=True` the tail slice ran forward and repeated most of the chunk. It now raises `ValueError`, matching the sibling options. + ## 0.27.9 ### Fixes diff --git a/test_unstructured/chunking/test_base.py b/test_unstructured/chunking/test_base.py index 7dab85e3b1..7b0a499255 100644 --- a/test_unstructured/chunking/test_base.py +++ b/test_unstructured/chunking/test_base.py @@ -171,6 +171,22 @@ def it_rejects_new_after_n_chars_for_n_less_than_zero(self, n_chars: int): ): ChunkingOptions(new_after_n_chars=n_chars)._validate() + @pytest.mark.parametrize("overlap", [-1, -5]) + def it_rejects_overlap_for_n_less_than_zero(self, overlap: int): + """A negative `overlap` silently corrupts chunk text, so reject it at validation. + + On the character-split path `_TextSplitter.__call__` computes the remainder as + `s[maxlen - overlap:]`, so a negative overlap starts the remainder *after* the end of the + fragment and drops that many characters of the document at every split boundary. With + `overlap_all=True`, `PreChunk.overlap_tail` computes `self._text[-overlap:]`, which for a + negative overlap is a forward slice that repeats nearly the whole chunk in the next one. + """ + with pytest.raises( + ValueError, + match=f"'overlap' argument must be >= 0, got {overlap}", + ): + ChunkingOptions(overlap=overlap)._validate() + def it_rejects_overlap_not_less_than_max_characters(self): with pytest.raises( ValueError, diff --git a/unstructured/__version__.py b/unstructured/__version__.py index 11757d2a5f..654374eb31 100644 --- a/unstructured/__version__.py +++ b/unstructured/__version__.py @@ -1 +1 @@ -__version__ = "0.27.9" # pragma: no cover +__version__ = "0.27.10" # pragma: no cover diff --git a/unstructured/chunking/base.py b/unstructured/chunking/base.py index c7d4ac5d47..c82c38b3a7 100644 --- a/unstructured/chunking/base.py +++ b/unstructured/chunking/base.py @@ -391,6 +391,15 @@ def _validate(self) -> None: " adjacent elements" ) + # -- a negative `overlap` is a caller mistake, not a smaller overlap: on the + # -- character-split path it moves the remainder's start past the end of the fragment and + # -- silently deletes that many characters from the document, and with `overlap_all=True` + # -- `_text[-overlap:]` becomes a forward slice that repeats most of the chunk instead of + # -- its tail. Reject it here, as `new_after_n_chars` already is. + overlap_arg = self._kwargs.get("overlap") + if overlap_arg is not None and overlap_arg < 0: + raise ValueError(f"'overlap' argument must be >= 0, got {overlap_arg}") + # -- overlap must be less than max-chars or the chunk text will never be consumed -- if self.overlap >= hard_max: raise ValueError(