Repository navigation
fix(chunker): reject negative token overlap - #3663
Conversation
Negative overlap produces a stride larger than the configured chunk size, silently skipping tokens between fixed-token windows. Reject negative values at the shared stride calculation point while preserving zero and positive overlap behaviour. Signed-off-by: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f3e5102d0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Validate negative overlap before delimiter handling so the result does not depend on document content or whether token windows are required. Keep the existing upper-bound validation lazy because it only applies when sliding windows are created. Signed-off-by: Lanre Shittu <136805224+Shizoqua@users.noreply.github.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Description
_window_step()calculates the fixed-token chunker's stride aschunk_token_size - chunk_overlap_token_size.When
chunk_overlap_token_sizeis negative, the stride becomes larger than the configured chunk size. This silently skips tokens between windows instead of producing the intended overlap.PR #3396 added upper-bound validation for overlap values greater than or equal to the chunk size. This change completes the boundary validation by rejecting negative values at the same shared stride calculation point.
Related Issues
No related issue.
Changes Made
chunk_overlap_token_sizevalues withValueError.The validation is placed in
_window_step()because it is the shared enforcement point for fixed-token window stride calculation.Checklist
Additional Notes
Verified against
mainatd692105. The regression test fails on the unmodified code and passes after the fix.The focused boundary tests pass, the chunker test suite passes excluding three pre-existing network-dependent tests, and
pre-commit run --all-filesandgit diff --checkcomplete successfully.