Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/chonkie/chunker/token.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,20 @@ def __init__(
super().__init__(tokenizer)
if chunk_size <= 0:
raise ValueError("chunk_size must be positive")
if isinstance(chunk_overlap, int) and chunk_overlap >= chunk_size:
raise ValueError("chunk_overlap must be less than chunk_size")

# Assign the values if they make sense
self.chunk_size = chunk_size
self.chunk_overlap = (
chunk_overlap if isinstance(chunk_overlap, int) else int(chunk_overlap * chunk_size)
)
# Validate the resolved overlap so a float chunk_overlap is held to the
# same invariant as an int one. A float that resolves to >= chunk_size
# would make the chunk stride (chunk_size - chunk_overlap) zero or
# negative, which crashes with range(..., 0) or silently drops all text.
if self.chunk_overlap < 0:
raise ValueError("chunk_overlap must be non-negative")
if self.chunk_overlap >= chunk_size:
raise ValueError("chunk_overlap must be less than chunk_size")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Checking self.chunk_overlap < 0 after resolving it to an integer can lead to inconsistent behavior for small negative float values due to truncation. For example, if chunk_overlap = -0.05 and chunk_size = 10, int(-0.05 * 10) resolves to 0, which silently bypasses the non-negative check. However, if chunk_size = 100, int(-0.05 * 100) resolves to -5, which correctly raises a ValueError.

To ensure consistent validation regardless of the chunk_size, we should validate that the input chunk_overlap is non-negative before resolving it.

Suggested change
if self.chunk_overlap < 0:
raise ValueError("chunk_overlap must be non-negative")
if self.chunk_overlap >= chunk_size:
raise ValueError("chunk_overlap must be less than chunk_size")
if chunk_overlap < 0:
raise ValueError("chunk_overlap must be non-negative")
if self.chunk_overlap >= chunk_size:
raise ValueError("chunk_overlap must be less than chunk_size")


self._use_multiprocessing = False

Expand Down
39 changes: 39 additions & 0 deletions tests/chunkers/test_token_chunker.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,3 +309,42 @@ def test_token_chunker_return_type(tiktokenizer: Encoding, sample_text: str) ->
chunks = chunker.chunk(sample_text)
assert all([type(chunk) is Chunk for chunk in chunks])
assert all([len(tiktokenizer.encode(chunk.text)) <= 512 for chunk in chunks])


def test_token_chunker_rejects_int_overlap_equal_to_chunk_size() -> None:
"""An int chunk_overlap equal to chunk_size is invalid (existing behavior)."""
with pytest.raises(ValueError, match="chunk_overlap must be less than chunk_size"):
TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=10)


def test_token_chunker_rejects_float_overlap_resolving_to_chunk_size() -> None:
"""A float chunk_overlap of 1.0 resolves to chunk_size and must be rejected.

Previously only the int path was validated, so this constructed a chunker with
a zero stride and chunk() raised 'range() arg 3 must not be zero'.
"""
with pytest.raises(ValueError, match="chunk_overlap must be less than chunk_size"):
TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=1.0)


def test_token_chunker_rejects_float_overlap_exceeding_chunk_size() -> None:
"""A float chunk_overlap > 1.0 resolves above chunk_size and must be rejected.

Previously this produced a negative stride and chunk() silently returned zero
chunks, dropping the entire input text.
"""
with pytest.raises(ValueError, match="chunk_overlap must be less than chunk_size"):
TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=1.5)


def test_token_chunker_accepts_valid_fractional_overlap_and_chunks() -> None:
"""A valid fractional float overlap resolves correctly and chunks non-empty text."""
chunker = TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=0.5)
assert chunker.chunk_overlap == 5
text = "x" * 40
chunks = chunker.chunk(text)
assert len(chunks) > 1
assert "".join(dict.fromkeys([c.text[0] for c in chunks])) == "x"
# Every original character is covered by the first chunk's start through the last chunk's end.
assert chunks[0].start_index == 0
assert chunks[-1].end_index == len(text)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

It would be beneficial to add test cases to verify that negative values for chunk_overlap (both integer and float) are correctly rejected by the chunker.

Suggested change
assert chunks[-1].end_index == len(text)
assert chunks[-1].end_index == len(text)
def test_token_chunker_rejects_negative_overlap() -> None:
"""Negative chunk_overlap values must be rejected."""
with pytest.raises(ValueError, match="chunk_overlap must be non-negative"):
TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=-1)
with pytest.raises(ValueError, match="chunk_overlap must be non-negative"):
TokenChunker(tokenizer="character", chunk_size=10, chunk_overlap=-0.1)