Knowledge sources accept chunk settings that silently store nothing or drop characters

[Lesereingrape]

Description

Every knowledge source accepts chunk_size / chunk_overlap combinations that cannot produce a valid chunking window. _chunk_text() then either stores zero documents or silently drops characters from the text it embeds, and neither case reports an error to the caller.

The chunking step is derived from the pair, and neither field is validated against the other (or against zero):

# lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py:19
    chunk_size: int = 4000
    chunk_overlap: int = 200

# lib/crewai/src/crewai/knowledge/source/base_knowledge_source.py:43
    def _chunk_text(self, text: str) -> list[str]:
        """Utility method to split text into chunks."""
        return [
            text[i : i + self.chunk_size]
            for i in range(0, len(text), self.chunk_size - self.chunk_overlap)
        ]

chunk_size - chunk_overlap is the range() step, so it is the only thing deciding what reaches storage. Three shapes are reachable through the public constructor:

chunk_size	chunk_overlap	step	what happens
100	200	-100	range(0, len(text), -100) is empty → 0 chunks → storage.save([])
100	100	0	add() raises ValueError: range() arg 3 must not be zero
10	-1	11	window advances by more than chunk_size → characters between chunks are never stored

The negative-overlap row is the one that worries me most: the run reports success, the knowledge base is written, and part of every document is simply unretrievable. The overlap > size row is the more likely typo (chunk_overlap=4000, chunk_size=2000), and it produces an empty knowledge base that still looks like a successful ingest.

Steps to Reproduce

No storage backend and no LLM needed — a stub storage object records what add() hands over.

from unittest.mock import MagicMock

from crewai.knowledge.source.string_knowledge_source import StringKnowledgeSource

ALPHABET = "0123456789ABCDEFGHIJ"


def case(chunk_size: int, chunk_overlap: int, text: str = ALPHABET) -> None:
    source = StringKnowledgeSource(
        content=text, chunk_size=chunk_size, chunk_overlap=chunk_overlap
    )
    storage = MagicMock()
    source.storage = storage
    try:
        source.add()
    except Exception as exc:  # noqa: BLE001
        print(f"({chunk_size}, {chunk_overlap}) add() raised {type(exc).__name__}: {exc}")
        return
    saved = storage.save.call_args[0][0]
    lost = "".join(sorted(set(text) - set("".join(saved))))
    print(f"({chunk_size}, {chunk_overlap}) saved {len(saved)} chunk(s) {saved!r} chars_lost={lost!r}")


case(100, 200, "x" * 5000)  # 5000 characters of content, nothing stored
case(100, 100)
case(10, 11)
case(10, -1)
case(0, 0)
case(-5, 0)

Actual behavior (measured, main @ 3831e8b, Python 3.13 / Windows)

(100, 200) saved 0 chunk(s) [] chars_lost='x'
(100, 100) add() raised ValueError: range() arg 3 must not be zero
(10, 11) saved 0 chunk(s) [] chars_lost='0123456789ABCDEFGHIJ'
(10, -1) saved 2 chunk(s) ['0123456789', 'BCDEFGHIJ'] chars_lost='A'
(0, 0) add() raised ValueError: range() arg 3 must not be zero
(-5, 0) saved 0 chunk(s) [] chars_lost='0123456789ABCDEFGHIJ'

All six constructions above succeed. The first, third and sixth store less than what was given, with no signal at all.

Note the (10, -1) output: '0123456789' + 'BCDEFGHIJ' — the character at index 10 is in neither chunk, so it never enters the embedding input.

Expected behavior

A source that cannot chunk its content should fail where the mistake is made — at construction, with a message naming both numbers — instead of writing an empty or partial knowledge base, or raising a raw range() complaint from deep inside add().

Cause

BaseKnowledgeSource declares chunk_size / chunk_overlap as plain int fields, so pydantic only checks the type. The relation that actually matters (0 <= chunk_overlap < chunk_size, chunk_size > 0) is implicit in the range() step at :47 and is never asserted.

_chunk_text() is also duplicated verbatim in the six subclasses that carry their own copy (csv_knowledge_source.py:50, excel_knowledge_source.py:180, json_knowledge_source.py:62, pdf_knowledge_source.py:62, string_knowledge_source.py:40, text_file_knowledge_source.py:42), so a per-subclass fix would have to be repeated. All eight sources inherit the field declarations from BaseKnowledgeSource, which is where a single guard belongs.

Possible solution

Validate the pair on the model, where both fields are visible, so the check no longer depends on when add() happens to run:

@model_validator(mode="after")
def _validate_chunk_settings(self) -> Self:
    """_chunk_text steps by chunk_size - chunk_overlap: zero makes range() raise, negative makes it yield no chunk."""
    if self.chunk_size <= 0:
        raise ValueError("chunk_size must be a positive integer")
    if self.chunk_overlap < 0:
        raise ValueError("chunk_overlap must be a non-negative integer")
    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

This is a narrowing, not a behavior change: every pair that produced correct chunks before still constructs, and only pairs that could not chunk at all are now rejected up front. The chunk_overlap >= chunk_size branch subsumes the step == 0 and step < 0 cases; the chunk_size <= 0 and chunk_overlap < 0 branches exist so the message names the bad field instead of blaming the other one.

Deduplicating the seven _chunk_text() copies looks like a separate, mechanical cleanup — I left it out to keep this change reviewable.

Environment

• crewAI: 1.15.22 (main @ 3831e8b)
• Python 3.13, Windows 11 (nothing OS specific)
• Verified with lib/crewai/tests/knowledge/: 6 new failing-before / passing-after cases plus controls; the only failures in that directory are two pre-existing test_docling_* ImportErrors (docling not installed locally) that reproduce identically on main.

Authored with an AI coding assistant. .github/CONTRIBUTING.md requires the llm-generated label for agent-authored contributions; external contributors cannot apply labels, so maintainers please add it.

[impartshadow]

The failure class here is silent data loss masquerading as success — the operation returns normally, but the artifact it produced is wrong or empty.

The root cause is exactly what the issue names: chunk_size / chunk_overlap are "plain int fields, so pydantic only checks the type." Pydantic validates shape, not relationship — nothing ever asserts chunk_overlap < chunk_size or that the derived step (chunk_size - chunk_overlap) is positive. That's the same shape as a lot of "config accepted, computation garbage" bugs: the type system says the input is fine, so the code downstream trusts it, and the first place the assumption actually gets checked is a Python builtin (range()) that either throws an opaque error or just runs with a nonsensical step.

The deterministic check that would have caught this belongs at the construction boundary, not inside _chunk_text(): a pydantic model_validator on BaseKnowledgeSource asserting 0 <= chunk_overlap < chunk_size (and chunk_size > 0), raising with both values named in the message. That converts all six repro cases into a same-line ValidationError instead of three different runtime behaviors (0 chunks, raw range() crash, partial-chunk data loss). A property test asserting ''.join(chunks) covers every character of the input for all valid (chunk_size, chunk_overlap) pairs would have caught the (10, -1) case specifically — that's the one that's genuinely invisible without such a test, since it doesn't crash or produce an empty result, it just quietly eats one character per window.

What I'm not certain of: whether any existing test suite already exercises _chunk_text with adversarial size/overlap pairs — if there's coverage that asserts total character count or reconstructs the original string, that's evidence this gap was known and left unguarded rather than never tested; if there's no such test anywhere in tests/knowledge/, that supports it being a pure validation gap.

Advisory SFA-2026-F989FDCFF9 · Full redacted write-up: https://impartshadow.github.io/echo-site/failure-cases/knowledge-sources-accept-chunk-settings-that-silentl-f989fdcff9.html
If this diagnosis is useful, react 👍 to this comment; if it is wrong, react 👎 — I use the verdict to choose what I investigate next.

[impartshadow]

The bounded implementation is now in #7617: one model_validator on BaseKnowledgeSource, with invalid-pair and valid-control vectors across string and file-backed sources. It deliberately leaves the duplicated _chunk_text() cleanup out of scope.

The remaining technical choice is explicit: reject chunk_size <= 0, chunk_overlap < 0, and chunk_overlap >= chunk_size at construction, or clamp the derived step as crewai-files does. I favor rejection because clamping silently changes the configured embedding windows—the same observability failure class as the bug.

Could a maintainer give an explicit verdict on reject-vs-clamp and, if rejection is right, approve the fork CI for #7617? The PR is mergeable, CodeRabbit found no actionable issue, and the focused local vector is 9/9 passing; the repository workflows are still waiting for maintainer approval rather than reporting a failure.