Skip to content

Commit bec0acc

Browse files
fix(chunking): measure can_combine in tokens in token mode
`PreChunk.can_combine()` sized its text with `len()` and compared the result to `combine_text_under_n_chars` and to `hard_max`. Both of those are token counts when `max_tokens` is given, so the comparison was characters against a token budget. Every sibling computation in the class -- `.will_fit()`, `._text_length`, `_TextSplitter` -- already routes through `ChunkingOptions.measure()`. A section of a few tokens is many more characters than the token budget, so `can_combine()` answered `False` for every short pre-chunk and `chunk_by_title()` emitted one chunk per section. With `max_tokens=60`, twelve ten-token sections came out as twelve chunks at 17% of the requested size, where `chunk_elements()` on the same elements produced two full ones. Raising `combine_text_under_n_chars` is not a way around it: values above `max_tokens` are rejected by option validation. Send both measurements through `measure()`. Character mode is unaffected, because `measure()` is `len()` there; the existing character-mode `can_combine()` cases still pass unchanged, and chunk output over a 3,000-case character-mode corpus is byte-identical.
1 parent 73b4295 commit bec0acc

6 files changed

Lines changed: 115 additions & 3 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
## 0.27.32-dev3
2+
3+
### Fixes
4+
5+
- **Token-based `chunk_by_title()` fills the chunking window again.** `PreChunk.can_combine()` sized its text with `len()`, so the `combine_text_under_n_chars` threshold and the hard-max check counted characters even when `max_tokens` selected token counting; every section looked too large to combine and became its own chunk (with `max_tokens=60`, twelve ten-token sections produced twelve chunks instead of two full ones). Both measurements now go through `ChunkingOptions.measure()`, as the rest of the class already does. Character-mode behavior is unchanged.
6+
17
## 0.27.32-dev2
28

39
### Enhancements
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import pytest
2+
3+
from test_unstructured.unit_utils import FixtureRequest, Mock, method_mock
4+
from unstructured.chunking.base import TokenCounter
5+
6+
7+
@pytest.fixture()
8+
def word_token_counter_(request: FixtureRequest) -> Mock:
9+
"""Count one token per whitespace-delimited word.
10+
11+
Deterministic and offline, so a test can state exact token counts without depending on
12+
tiktoken being installed or on the contents of a particular encoding.
13+
"""
14+
return method_mock(
15+
request,
16+
TokenCounter,
17+
"count",
18+
side_effect=lambda _, text: len(text.split()),
19+
)

‎test_unstructured/chunking/test_base.py‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from lxml.html import fragment_fromstring
1515

1616
import unstructured.chunking.base as chunking_base
17+
from test_unstructured.unit_utils import Mock
1718
from unstructured.chunking.base import (
1819
ChunkingOptions,
1920
PreChunk,
@@ -879,6 +880,50 @@ def it_knows_when_it_can_combine_itself_with_another_PreChunk_instance(
879880

880881
assert pre_chunk.can_combine(next_pre_chunk) is expected_value
881882

883+
@pytest.mark.parametrize(
884+
("max_tokens", "combine_text_under_n_chars", "expected_value"),
885+
[
886+
# Will exactly fit:
887+
# - text = 7 tokens < combine_text_under_n_chars
888+
# - pre_chunk + separator + next_pre_chunk_text = 7 + 0 + 5 = 12 <= max_tokens
889+
(12, 8, True),
890+
# -- already exceeds the combine threshold, which is also counted in tokens --
891+
(12, 7, False),
892+
# -- would exceed the hard-max chunking-window threshold --
893+
(11, 8, False),
894+
],
895+
)
896+
def it_measures_in_tokens_when_it_can_combine_itself_with_another_PreChunk_instance(
897+
self,
898+
max_tokens: int,
899+
combine_text_under_n_chars: int,
900+
expected_value: bool,
901+
word_token_counter_: Mock,
902+
):
903+
"""Both thresholds are token counts in token mode, not character counts.
904+
905+
Measuring the text in characters here made `.can_combine()` answer `False` for every
906+
short pre-chunk, because even a handful of tokens is more characters than the token
907+
budget. `chunk_by_title()` then emitted one chunk per section.
908+
"""
909+
opts = ChunkingOptions(
910+
max_tokens=max_tokens,
911+
tokenizer="cl100k_base",
912+
combine_text_under_n_chars=combine_text_under_n_chars,
913+
)
914+
pre_chunk = PreChunk(
915+
[Text("Lorem ipsum dolor sit amet consectetur adipiscing.")], # -- 7 tokens, 50 chars
916+
overlap_prefix="",
917+
opts=opts,
918+
)
919+
next_pre_chunk = PreChunk(
920+
[Text("In rhoncus sum sed lectus.")], # -- 5 tokens, 26 chars
921+
overlap_prefix="",
922+
opts=opts,
923+
)
924+
925+
assert pre_chunk.can_combine(next_pre_chunk) is expected_value
926+
882927
def it_does_not_combine_when_either_pre_chunk_contains_a_table(self):
883928
opts = ChunkingOptions(max_characters=500, combine_text_under_n_chars=500)
884929
text_pre_chunk = PreChunk([Text("hello")], overlap_prefix="", opts=opts)

‎test_unstructured/chunking/test_title.py‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
from test_unstructured.unit_utils import FixtureRequest, Mock, function_mock, input_path
1212
from unstructured.chunking.base import CHUNK_MULTI_PAGE_DEFAULT
13+
from unstructured.chunking.basic import chunk_elements
1314
from unstructured.chunking.title import _ByTitleChunkingOptions, chunk_by_title
1415
from unstructured.documents.coordinates import CoordinateSystem
1516
from unstructured.documents.elements import (
@@ -888,3 +889,42 @@ def it_applies_token_based_overlap_in_split_chunks(self, _tiktoken_installed: No
888889
for chunk in chunks:
889890
token_count = len(enc.encode(chunk.text))
890891
assert token_count <= 12, f"Chunk exceeded token limit: {token_count} tokens"
892+
893+
def it_fills_the_token_budget_when_combining_short_sections(self, word_token_counter_: Mock):
894+
"""Short sections combine up to `max_tokens`, like `chunk_elements()` does.
895+
896+
`combine_text_under_n_chars` defaults to `max_tokens` in token mode, so a section had to
897+
be under that many *tokens* to be combined. Measuring it in characters instead put every
898+
section over the threshold and suppressed combining altogether, leaving one chunk per
899+
section at a fraction of the requested chunk size.
900+
"""
901+
elements: list[Element] = []
902+
for idx in range(12):
903+
# -- each section is 2 + 8 == 10 tokens, a sixth of the budget --
904+
elements.append(Title(f"Section {idx}"))
905+
elements.append(Text("alpha beta gamma delta epsilon zeta seven eight"))
906+
907+
chunks = chunk_by_title(elements, max_tokens=60, tokenizer="cl100k_base")
908+
909+
assert [len(chunk.text.split()) for chunk in chunks] == [60, 60]
910+
# -- and the section boundaries cost nothing relative to ignoring them entirely --
911+
assert [chunk.text for chunk in chunks] == [
912+
chunk.text for chunk in chunk_elements(elements, max_tokens=60, tokenizer="cl100k_base")
913+
]
914+
915+
def it_still_measures_the_combine_threshold_in_characters_in_character_mode(self):
916+
"""Character mode is unaffected: `combine_text_under_n_chars` stays a character count."""
917+
elements: list[Element] = [
918+
Title("Alpha"),
919+
Text("one two three four five"), # -- 5 tokens but 23 characters --
920+
Title("Bravo"),
921+
Text("six seven eight nine ten"),
922+
]
923+
924+
chunks = chunk_by_title(elements, max_characters=200, combine_text_under_n_chars=20)
925+
926+
# -- 29 characters is over the 20-character threshold, so no combining --
927+
assert [chunk.text for chunk in chunks] == [
928+
"Alpha\n\none two three four five",
929+
"Bravo\n\nsix seven eight nine ten",
930+
]

‎unstructured/__version__.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
__version__ = "0.27.32-dev2" # pragma: no cover
1+
__version__ = "0.27.32-dev3" # pragma: no cover

‎unstructured/chunking/base.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -771,13 +771,15 @@ def can_combine(self, pre_chunk: PreChunk) -> bool:
771771
self._elements, pre_chunk._elements
772772
):
773773
return False
774-
if len(self._text) >= self._opts.combine_text_under_n_chars:
774+
# -- both thresholds below are expressed in the configured units, so the text has to be
775+
# -- sized in those units too; `measure()` is `len()` in character mode --
776+
if self._opts.measure(self._text) >= self._opts.combine_text_under_n_chars:
775777
return False
776778
# -- avoid duplicating length computations by doing a trial-combine which is just as
777779
# -- efficient and definitely more robust than hoping two different computations of combined
778780
# -- length continue to get the same answer as the code evolves. Only possible because
779781
# -- `.combine()` is non-mutating.
780-
combined_len = len(self.combine(pre_chunk)._text)
782+
combined_len = self._opts.measure(self.combine(pre_chunk)._text)
781783

782784
return combined_len <= self._opts.hard_max
783785

0 commit comments

Comments
 (0)