Skip to content

Commit 8c43c01

Browse files
chuenchen309claude
andcommitted
fix: reject chunk_overlap >= chunk_size in knowledge sources instead of crashing/dropping content
BaseKnowledgeSource._chunk_text() (and 6 identical copies of it duplicated across string/json/pdf/text_file/csv/excel KnowledgeSource subclasses) compute the slicing step as chunk_size - chunk_overlap with no validation: range(0, len(text), chunk_size - chunk_overlap) chunk_overlap == chunk_size -> step is 0 -> ValueError("range() arg 3 must not be zero"), a confusing error unrelated to the actual misconfiguration. chunk_overlap > chunk_size -> step is negative -> range(0, len(text), neg) with start(0) < stop(len(text)) silently yields nothing -> _chunk_text returns [] -> the source's entire content is dropped with no error, no warning, nothing indexed. Verified both independently with a real (non-mocked) StringKnowledgeSource instance before this fix: chunk_overlap=chunk_size raises the confusing range() ValueError; chunk_overlap>chunk_size raises nothing and silently produces zero chunks. Fix: add a single `@model_validator(mode="after")` on BaseKnowledgeSource that rejects chunk_overlap >= chunk_size at construction time with a clear message. Since every one of the 7 duplicated _chunk_text implementations reads self.chunk_size/self.chunk_overlap from the same inherited Pydantic fields, and Pydantic validators are inherited by subclasses, this one change protects all 7 call sites without needing to touch each file's duplicated _chunk_text individually. Confirmed via direct instantiation of StringKnowledgeSource with the bad configs both now raise ValidationError at construction time, before any chunking is ever attempted. Added tests/knowledge/test_base_knowledge_source_chunk_overlap.py: equal values rejected, overlap > size rejected, a valid overlap < size accepted, and the library's own defaults (chunk_size=4000, chunk_overlap=200) pass the new check. Confirmed red->green (reverting the source change reproduces "DID NOT RAISE ValueError" for both bad-config tests). Full tests/knowledge/ suite: 58 passed, no regressions. ruff + mypy clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 0e5d0ec commit 8c43c01

2 files changed

Lines changed: 41 additions & 1 deletion

File tree

lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,8 @@
22
from typing import Any
33

44
import numpy as np
5-
from pydantic import BaseModel, ConfigDict, Field
5+
from pydantic import BaseModel, ConfigDict, Field, model_validator
6+
from typing_extensions import Self
67

78
from crewai.knowledge.storage.base_knowledge_storage import BaseKnowledgeStorage
89
from crewai.knowledge.storage.knowledge_storage import KnowledgeStorage
@@ -28,6 +29,21 @@ class BaseKnowledgeSource(BaseModel, ABC):
2829
metadata: dict[str, Any] = Field(default_factory=dict) # Currently unused
2930
collection_name: str | None = Field(default=None)
3031

32+
@model_validator(mode="after")
33+
def _validate_chunk_overlap(self) -> Self:
34+
"""Reject a chunk_overlap that would break _chunk_text's slicing step.
35+
36+
step = chunk_size - chunk_overlap: zero raises ValueError("range() arg 3
37+
must not be zero"); negative silently makes range() empty, dropping all
38+
content with no error at all.
39+
"""
40+
if self.chunk_overlap >= self.chunk_size:
41+
raise ValueError(
42+
f"chunk_overlap ({self.chunk_overlap}) must be smaller than "
43+
f"chunk_size ({self.chunk_size})."
44+
)
45+
return self
46+
3147
@abstractmethod
3248
def validate_content(self) -> Any:
3349
"""Load and preprocess content from the source."""
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
"""Test BaseKnowledgeSource rejects a chunk_overlap that would break _chunk_text's slicing step."""
2+
3+
import pytest
4+
from crewai.knowledge.source.string_knowledge_source import StringKnowledgeSource
5+
6+
7+
def test_chunk_overlap_equal_to_chunk_size_is_rejected():
8+
with pytest.raises(ValueError, match="chunk_overlap"):
9+
StringKnowledgeSource(content="hello world", chunk_size=50, chunk_overlap=50)
10+
11+
12+
def test_chunk_overlap_greater_than_chunk_size_is_rejected():
13+
with pytest.raises(ValueError, match="chunk_overlap"):
14+
StringKnowledgeSource(content="hello world", chunk_size=50, chunk_overlap=60)
15+
16+
17+
def test_chunk_overlap_smaller_than_chunk_size_is_accepted():
18+
source = StringKnowledgeSource(content="hello world", chunk_size=50, chunk_overlap=10)
19+
assert source.chunk_overlap == 10
20+
21+
22+
def test_default_chunk_size_and_overlap_are_valid():
23+
source = StringKnowledgeSource(content="hello world")
24+
assert source.chunk_overlap < source.chunk_size

0 commit comments

Comments
 (0)