Skip to content

fix(chunker): reject negative token overlap - #3663

Merged
danielaskdd merged 2 commits into
HKUDS:mainfrom
Shizoqua:fix/reject-negative-chunk-overlap
Aug 18, 2026
Merged

danielaskdd merged 2 commits into
HKUDS:mainfrom
Shizoqua:fix/reject-negative-chunk-overlap

Conversation

@Shizoqua

Copy link
Copy Markdown
Contributor

Description

_window_step() calculates the fixed-token chunker's stride as chunk_token_size - chunk_overlap_token_size.

When chunk_overlap_token_size is 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

  • Reject negative chunk_overlap_token_size values with ValueError.
  • Add a regression test confirming negative overlap is rejected.
  • Add control coverage confirming zero and positive overlap still produce complete, gap-free token coverage.

The validation is placed in _window_step() because it is the shared enforcement point for fixed-token window stride calculation.

Checklist

  • Changes tested locally
  • Code reviewed
  • Documentation updated (if necessary)
  • Unit tests added (if applicable)

Additional Notes

Verified against main at d692105. 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-files and git diff --check complete successfully.

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>
@danielaskdd

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread lightrag/chunker/token_size.py Resolved
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>
@danielaskdd

Copy link
Copy Markdown
Collaborator

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 15788b3379

ℹ️ 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".

@danielaskdd
danielaskdd merged commit 119a064 into HKUDS:main Aug 18, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants