diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a18a66b74..47f7271960 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.27.24 + +### 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.23 ### Fixes diff --git a/test_unstructured/chunking/test_base.py b/test_unstructured/chunking/test_base.py index fcf0099b32..4c3bcd9c91 100644 --- a/test_unstructured/chunking/test_base.py +++ b/test_unstructured/chunking/test_base.py @@ -172,6 +172,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 1f3fae62c1..48bb3c3063 100644 --- a/unstructured/__version__.py +++ b/unstructured/__version__.py @@ -1 +1 @@ -__version__ = "0.27.23" # pragma: no cover +__version__ = "0.27.24" # pragma: no cover diff --git a/unstructured/chunking/base.py b/unstructured/chunking/base.py index f2a6c1fd9f..5bbcc75c53 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(