Repository navigation
fix(chunker): preserve URLs in CharacterTextChunker (#2510) - #2515
zichen0116 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
🤖 Open Code ReviewTarget: PR #2515 🔍 OpenCodeReview found 3 issue(s) in this PR. 1.
|
Description
CharacterTextChunkerreplaces URLs with__URL_<n>__placeholders beforesplitting, but
RecursiveCharacterTextSplittercan cut right through aplaceholder (e.g.
__URL/_0__landing in different chunks). The URL canthen no longer be restored: it disappears from the output and placeholder
fragments leak into the chunks.
The fix takes the first approach suggested in the issue: when the input
contains URLs, delegate to
SimpleTextSplitterwith the samechunk_size/chunk_overlap. It already keeps placeholders indivisible(see #2228), so this reuses the tested protection instead of adding a
second boundary-alignment implementation. Inputs without URLs keep the
previous recursive-splitter path byte-for-byte.
Related Issue (Required): Fixes #2510
Type of change
How Has This Been Tested?
New
tests/chunkers/test_charactertext_chunker.py(5 tests):test_url_preserved_with_small_chunk_sizeis the exact reproduction fromthe issue — confirmed failing before the fix (2 of the URL tests fail on
unpatched code) and passing after. Also covers overlap, a URL longer than
chunk_size(kept whole, may oversize one chunk), multiple URLs, and ano-URL test pinning the recursive-splitter output unchanged.
ruff checkandruff format --checkpass on both changed files.Checklist