fix: reject chunk_overlap >= chunk_size in knowledge sources instead of crashing/dropping content

[chuenchen309]

Summary

BaseKnowledgeSource._chunk_text() — and 6 byte-for-byte identical copies of
it duplicated across StringKnowledgeSource, JSONKnowledgeSource,
PDFKnowledgeSource, TextFileKnowledgeSource, CSVKnowledgeSource, and
ExcelKnowledgeSource — compute the slicing step with no validation:

range(0, len(text), self.chunk_size - self.chunk_overlap)

• chunk_overlap == chunk_size → step is 0 → ValueError("range() arg 3 must not be zero"), a confusing error with no indication it's a
  chunk_size/chunk_overlap misconfiguration.
• chunk_overlap > chunk_size → step is negative → range(0, len(text), negative) with start(0) < stop(len(text)) silently yields nothing →
  _chunk_text returns [] → the entire source content is dropped, no
  error, no warning, nothing gets indexed.

Verified both independently with a real StringKnowledgeSource instance
(no mocking) before this fix — confirmed the exact ValueError message for
the equal case, and confirmed zero chunks / no exception at all for the
greater-than case.

Fix

Added a single @model_validator(mode="after") on BaseKnowledgeSource
that rejects chunk_overlap >= chunk_size at construction time with a clear
message. 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 — so this one
change on the base class protects all 7 call sites without touching each
file's duplicated method individually. Confirmed by direct instantiation:
StringKnowledgeSource(..., chunk_size=50, chunk_overlap=50) and
chunk_overlap=60 both now raise ValidationError immediately, before any
chunking is attempted.

Testing

Added tests/knowledge/test_base_knowledge_source_chunk_overlap.py:

• chunk_overlap == chunk_size → rejected
• chunk_overlap > chunk_size → rejected
• chunk_overlap < chunk_size → accepted
• library defaults (chunk_size=4000, chunk_overlap=200) → accepted

Confirmed red→green: reverting the source change reproduces "DID NOT RAISE
ValueError" for both bad-config tests; reapplying passes all 4.

Full tests/knowledge/ suite: 58 passed, no regressions. ruff check +
ruff format --check + mypy all clean.

Note on overlap

Open PR #5899 also touches base_knowledge_source.py (adds a
description= to the metadata field and threads metadata through
_save_documents/_asave_documents) — different lines, no functional
overlap with this change, but flagging it since both touch the same file.

AI-Generated disclosure

Implemented with Claude Code (Anthropic) — I independently reproduced both
failure modes with a real (non-mocked) knowledge source instance, verified
the fix protects all 7 duplicated call sites via Pydantic's validator
inheritance, and ran the full test suite before submitting.

Per CONTRIBUTING.md, AI-assisted PRs should carry the llm-generated label.
As an external contributor I don't have permission to self-apply labels
(confirmed via gh pr edit --add-label, GraphQL permission error) — happy
for a maintainer to apply it.

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

Chunk overlap validation

Layer / File(s)	Summary
Validate chunk overlap configuration lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py, lib/crewai/tests/knowledge/test_base_knowledge_source_chunk_overlap.py	Adds a post-validation check requiring chunk_overlap to be smaller than chunk_size, with tests for rejected, accepted, and default configurations.

Suggested reviewers: greysonlalonde

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: validating chunk_overlap against chunk_size to prevent invalid chunking behavior.
Description check	✅ Passed	The description directly explains the validation fix, affected knowledge sources, and added tests.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py (1)

32-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding bounds validation for chunk_size and chunk_overlap.

The current validator effectively prevents chunk_overlap >= chunk_size. As a complementary improvement to this configuration validation, consider ensuring that chunk_size and chunk_overlap are strictly non-negative by using Pydantic's Field constraints. This would prevent mathematically invalid values (like negative numbers) from passing model creation.

For example, updating the field definitions (lines 20-21) to:

    chunk_size: int = Field(default=4000, gt=0)
    chunk_overlap: int = Field(default=200, ge=0)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py` around lines
32 - 46, Update the chunk_size and chunk_overlap field definitions in the
knowledge source model to enforce chunk_size > 0 and chunk_overlap >= 0 using
Pydantic Field constraints, while preserving their existing defaults. Keep
_validate_chunk_overlap responsible for ensuring chunk_overlap remains smaller
than chunk_size.

lib/crewai/tests/knowledge/test_base_knowledge_source_chunk_overlap.py (1)

7-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use Pydantic's ValidationError for more precise exception testing.

Although Pydantic's ValidationError inherits from Python's built-in ValueError (which allows this test to pass), it is more idiomatic and precise to explicitly test for ValidationError. This ensures that you are strictly verifying Pydantic's validation behavior and not inadvertently passing the test if a standard ValueError were raised by an internal method.

♻️ Proposed refactor

+from pydantic import ValidationError
+
 def test_chunk_overlap_equal_to_chunk_size_is_rejected():
-    with pytest.raises(ValueError, match="chunk_overlap"):
+    with pytest.raises(ValidationError, match="chunk_overlap"):
         StringKnowledgeSource(content="hello world", chunk_size=50, chunk_overlap=50)
 
 
 def test_chunk_overlap_greater_than_chunk_size_is_rejected():
-    with pytest.raises(ValueError, match="chunk_overlap"):
+    with pytest.raises(ValidationError, match="chunk_overlap"):
         StringKnowledgeSource(content="hello world", chunk_size=50, chunk_overlap=60)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/crewai/tests/knowledge/test_base_knowledge_source_chunk_overlap.py`
around lines 7 - 14, Update both chunk-overlap rejection tests,
test_chunk_overlap_equal_to_chunk_size_is_rejected and
test_chunk_overlap_greater_than_chunk_size_is_rejected, to assert Pydantic’s
ValidationError instead of the broader ValueError, and import ValidationError
from the appropriate Pydantic package.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py`:
- Around line 32-46: Update the chunk_size and chunk_overlap field definitions
in the knowledge source model to enforce chunk_size > 0 and chunk_overlap >= 0
using Pydantic Field constraints, while preserving their existing defaults. Keep
_validate_chunk_overlap responsible for ensuring chunk_overlap remains smaller
than chunk_size.

In `@lib/crewai/tests/knowledge/test_base_knowledge_source_chunk_overlap.py`:
- Around line 7-14: Update both chunk-overlap rejection tests,
test_chunk_overlap_equal_to_chunk_size_is_rejected and
test_chunk_overlap_greater_than_chunk_size_is_rejected, to assert Pydantic’s
ValidationError instead of the broader ValueError, and import ValidationError
from the appropriate Pydantic package.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5346bfa5-6fd2-4d32-8fc9-88a2af8df143

📥 Commits

Reviewing files that changed from the base of the PR and between 0e5d0ec and 8c43c01.

📒 Files selected for processing (2)

• lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py
• lib/crewai/tests/knowledge/test_base_knowledge_source_chunk_overlap.py