fix: validate chunk_overlap < chunk_size in knowledge sources - #6784
Open
NishchayMahor wants to merge 1 commit into
Open
fix: validate chunk_overlap < chunk_size in knowledge sources#6784NishchayMahor wants to merge 1 commit into
NishchayMahor wants to merge 1 commit into
Conversation
_chunk_text steps by chunk_size - chunk_overlap. When chunk_overlap >= chunk_size the step is <= 0, so the document is silently dropped (empty range) or ingestion crashes with 'range() arg 3 must not be zero'. Both fields are user-settable with no cross-field check. Add a model validator on BaseKnowledgeSource so every source fails fast with a clear message.
📝 WalkthroughWalkthroughChangesKnowledge source validation
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py`:
- Around line 32-46: Update _validate_chunk_settings in the base knowledge
source model to require chunk_size > 0 and chunk_overlap >= 0, while preserving
the existing requirement that chunk_overlap is smaller than chunk_size. Raise
clear ValueError messages for each invalid setting, and add regression tests
covering non-positive chunk_size and negative chunk_overlap.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4a7ff73f-89e7-4b42-9921-404e5ac11c59
📒 Files selected for processing (2)
lib/crewai/src/crewai/knowledge/source/base_knowledge_source.pylib/crewai/tests/knowledge/test_knowledge.py
Comment on lines
+32
to
+46
| @model_validator(mode="after") | ||
| def _validate_chunk_settings(self) -> Self: | ||
| """Ensure chunk_overlap is smaller than chunk_size. | ||
|
|
||
| Otherwise the step in ``_chunk_text`` (``chunk_size - chunk_overlap``) | ||
| becomes zero or negative, which either drops the document silently | ||
| (empty range) or raises ``ValueError: range() arg 3 must not be zero``. | ||
| """ | ||
| if self.chunk_overlap >= self.chunk_size: | ||
| raise ValueError( | ||
| f"chunk_overlap ({self.chunk_overlap}) must be smaller than " | ||
| f"chunk_size ({self.chunk_size})." | ||
| ) | ||
| return self | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | rg '(^|/)base_knowledge_source\.py$|README|pydantic|requirements|poetry|pyproject|pytest|test.*knowledge'
echo "== file excerpt =="
if [ -f lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py ]; then
nl -ba lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py | sed -n '1,140p'
fi
echo "== related tests/usages =="
rg -n "BaseKnowledgeSource|StringKnowledgeSource|chunk_(size|overlap)|model_validator|_validate_chunk_settings" -S . || true
echo "== pyproject/pip hints =="
for f in pyproject.toml requirements.txt setup.py setup.cfg poetry.lock; do
[ -f "$f" ] && { echo "-- $f"; sed -n '1,220p' "$f"; }
doneRepository: crewAIInc/crewAI
Length of output: 8740
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== base_knowledge_source.py excerpt =="
if [ -f lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py ]; then
python3 - <<'PY'
from pathlib import Path
p=Path("lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py")
for i,line in enumerate(p.read_text().splitlines(),1):
if 1 <= i <= 120:
print(f"{i:4}\t{line}")
PY
fi
echo "== related tests/usages =="
rg -n "BaseKnowledgeSource|StringKnowledgeSource|chunk_(size|overlap)|model_validator|_validate_chunk_settings|step = chunk_size - chunk_overlap|range\\(" -S . || true
echo "== pyproject hints =="
python3 - <<'PY'
from pathlib import Path
for f in ["lib/crewai/pyproject.toml","pyproject.toml"]:
p=Path(f)
if p.exists():
print(f"-- {f}")
text=p.read_text()
for line in text.splitlines():
if "pydantic" in line.lower() or line.startswith("name") or line.startswith("version") or line.startswith("[project"):
print(line)
PYRepository: crewAIInc/crewAI
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file line numbers with python/readlines =="
python3 - <<'PY'
from pathlib import Path
p=Path("lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py")
for i,line in enumerate(p.read_text().splitlines(),1):
if 1 <= i <= 120:
print(f"{i:4}\t{line}")
PY
echo "== string_knowledge_source.py =="
if [ -f lib/crewai/src/crewai/knowledge/source/string_knowledge_source.py ]; then
python3 - <<'PY'
from pathlib import Path
p=Path("lib/crewai/src/crewai/knowledge/source/string_knowledge_source.py")
for i,line in enumerate(p.read_text().splitlines(),1):
print(f"{i:4}\t{line}")
PY
fi
echo "== focused tests in lib/crewai/tests/knowledge =="
python3 - <<'PY'
import subprocess
files = ["test_knowledge.py","test_async_knowledge.py","test_knowledge_storage_integration.py","test_storage_factory.py",
"../utilities/test_pydantic_schema_utils.py"]
for name in files:
if Path(name).exists():
print(f"\n-- {name} relevant matches:")
subprocess.run(["grep","-n","-E","chunk_(size|overlap)|BaseKnowledgeSource|StringKnowledgeSource|negative|gt=|ge=|validation|ValidationError"], str(name), text=True)
PY
echo "== behavioral probe for range behavior =="
python3 - <<'PY'
def chunks(text, chunk_size, chunk_overlap):
if chunk_size > 0:
step = chunk_size - chunk_overlap
return [text[i:i + chunk_size] for i in range(0, len(text), step)]
return []
print(chunks("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 100, -1))
print(chunks("ABCDEFGHIJKLMNOPQRSTUVWXYZ", 0, 0))
print(chunks("ABCDEFGHIJKLMNOPQRSTUVWXYZ", -10, 0))
PYRepository: crewAIInc/crewAI
Length of output: 5991
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import subprocess
for name in [
"lib/crewai/tests/knowledge/test_knowledge.py",
"lib/crewai/tests/knowledge/test_async_knowledge.py",
"lib/crewai/tests/knowledge/test_knowledge_storage_integration.py",
"lib/crewai/tests/knowledge/test_storage_factory.py",
"lib/crewai/tests/utilities/test_pydantic_schema_utils.py",
]:
p = Path(name)
if p.exists():
print(f"\n-- {name} relevant matches:")
subprocess.run(["grep","-n","-E","chunk_(size|overlap)|BaseKnowledgeSource|StringKnowledgeSource|negative|gt=|ge=|validation|ValidationError"], name)
print("\n-- behavioral probe for range behavior:")
def chunks(text, chunk_size, chunk_overlap):
if chunk_size <= 0:
return []
return [text[i:i + chunk_size] for i in range(0, len(text), chunk_size - chunk_overlap)]
cases = [
("ABCDE", 5, 0),
("ABCDE", 5, -1),
("ABCDEFGHIJKLMNO", 5, -1),
("ABCDEFGHIJKLMNO", 3, -1),
("ABCDEFGHIJKLMNO", 1, -1),
("ABCDE", 3, -10),
("ABCDE", 0, 0),
("ABCDE", -1, 0),
]
for text, cs, co in cases:
step = cs - co if cs > 0 else None
result = chunks(text, cs, co)
print({"text_len": len(text), "chunk_size": cs, "chunk_overlap": co, "step": step, "chunks": result, "skipped": [(len(result) - sum(len(c) for c in result)<len(text))]})
PYRepository: crewAIInc/crewAI
Length of output: 561
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
files = [
"lib/crewai/tests/knowledge/test_knowledge.py",
"lib/crewai/tests/knowledge/test_async_knowledge.py",
"lib/crewai/tests/knowledge/test_knowledge_storage_integration.py",
"lib/crewai/tests/knowledge/test_storage_factory.py",
"lib/crewai/tests/utilities/test_pydantic_schema_utils.py",
]
pattern = "chunk_(size|overlap)|BaseKnowledgeSource|StringKnowledgeSource|negative|gt=|ge=|validation|ValidationError"
for path in files:
p = Path(path)
if not p.exists():
continue
print(f"\n-- {path} relevant matches ({':'.join(map(str,parts))}):")
parts = (pattern, "-n", path)
import subprocess
subprocess.run(["grep"] + list(parts), check=False)
PY
python3 - <<'PY'
print("-- behavioral probe for range behavior:")
text = "ABCDEFGHIJKLMNO"
def chunks(text, chunk_size, chunk_overlap):
if chunk_size <= 0:
return []
return [text[i:i + chunk_size] for i in range(0, len(text), chunk_size - chunk_overlap)]
cases = [
("ABCDE", 5, 0),
("ABCDE", 5, -1),
("ABCDEFGHIJKLMNO", 5, -1),
("ABCDEFGHIJKLMNO", 3, -1),
("ABCDEFGHIJKLMNO", 1, -1),
("ABCDE", 3, -10),
("ABCDE", 0, 0),
("ABCDE", -1, 0),
]
for text, cs, co in cases:
result = chunks(text, cs, co)
print({"chunk_size": cs, "chunk_overlap": co, "step": cs - co, "chunks": result, "covered_len": sum(len(c) for c in result), "input_len": len(text)})
PYRepository: crewAIInc/crewAI
Length of output: 267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 <<'PY'
from pathlib import Path
files = [
"lib/crewai/tests/knowledge/test_knowledge.py",
"lib/crewai/tests/knowledge/test_async_knowledge.py",
"lib/crewai/tests/knowledge/test_knowledge_storage_integration.py",
"lib/crewai/tests/knowledge/test_storage_factory.py",
"lib/crewai/tests/utilities/test_pydantic_schema_utils.py",
]
for path in files:
p = Path(path)
if not p.exists():
continue
print(f"\n-- {path} relevant matches:")
for i, line in enumerate(p.read_text().splitlines(), 1):
if "chunk_size" in line or "chunk_overlap" in line or "BaseKnowledgeSource" in line or "StringKnowledgeSource" in line or "negative" in line or "gt=" in line or "ge=" in line or "validation" in line or "ValidationError" in line:
print(f"{i}:{line}")
print("\n-- behavioral probe for range behavior:")
text = "ABCDEFGHIJKLMNO"
def chunks(text, chunk_size, chunk_overlap):
if chunk_size <= 0:
return []
return [text[i:i + chunk_size] for i in range(0, len(text), chunk_size - chunk_overlap)]
cases = [
("ABCDE", 5, 0),
("ABCDE", 5, -1),
("ABCDEFGHIJKLMNO", 5, -1),
("ABCDEFGHIJKLMNO", 3, -1),
("ABCDEFGHIJKLMNO", 1, -1),
("ABCDE", 3, -10),
("ABCDE", 0, 0),
("ABCDE", -1, 0),
]
for text, cs, co in cases:
result = chunks(text, cs, co)
print({"chunk_size": cs, "chunk_overlap": co, "step": cs - co, "chunks": result, "covered_len": sum(len(c) for c in result), "input_len": len(text)})
PYRepository: crewAIInc/crewAI
Length of output: 3609
Reject invalid chunk settings.
chunk_overlap < 0 currently passes validation and makes chunk_size - chunk_overlap larger than chunk_size, so _chunk_text skips characters between chunks. chunk_size <= 0 also produces no chunks. Enforce chunk_size > 0 and chunk_overlap >= 0 alongside the existing overlap check, and add regression tests for these cases.
🤖 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 _validate_chunk_settings in the base knowledge source model to
require chunk_size > 0 and chunk_overlap >= 0, while preserving the existing
requirement that chunk_overlap is smaller than chunk_size. Raise clear
ValueError messages for each invalid setting, and add regression tests covering
non-positive chunk_size and negative chunk_overlap.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Knowledge sources chunk text with:
chunk_sizeandchunk_overlapare both user-settable with no cross-field check, so whenchunk_overlap >= chunk_sizethe step is<= 0:chunk_overlap > chunk_size→ negative step → empty range → the document is silently dropped (never embedded or saved).chunk_overlap == chunk_size→ step0→ValueError: range() arg 3 must not be zerocrashes ingestion with an opaque message.Fix
Add a
model_validatoronBaseKnowledgeSource(where both fields are defined, so it covers every source type) that fails fast with a clear message:Valid configurations — including the defaults (
chunk_size=4000,chunk_overlap=200) — are unaffected.Testing
Added
test_knowledge_source_rejects_overlap_not_smaller_than_size(both>and==cases raise; a valid config still constructs). Fails onmain, passes with the fix. ruff clean.This fix was developed with AI assistance; I verified the silent-drop and crash behaviors and the fix, and reviewed every line.