Skip to content

Fix overlapped chunks exceeding chunk_size when separators count as tokens#25

Open
lntutor wants to merge 1 commit into
isaacus-dev:mainfrom
lntutor:fix/overlap-exceeds-chunk-size
Open

Fix overlapped chunks exceeding chunk_size when separators count as tokens#25
lntutor wants to merge 1 commit into
isaacus-dev:mainfrom
lntutor:fix/overlap-exceeds-chunk-size

Conversation

@lntutor

@lntutor lntutor commented Jul 19, 2026

Copy link
Copy Markdown

What's broken

When overlap is enabled, output chunks are built by spanning several subchunks by offset:

suboffsets[start][0] : suboffsets[end][1]

That span re-includes the whitespace separators between subchunks (which were stripped when the subchunks were formed). With a whitespace-significant token counter (e.g. a character counter), those separators count as tokens and push the merged chunk past chunk_size, breaking the documented maximum:

semchunk.chunk("aa  bb  cc  dd", 4, len, overlap=0.5, memoize=False)
# ['aa  bb', 'bb  cc', 'cc  dd']   -- each 6 tokens > chunk_size 4

The code's own NOTE comments use floor (not ceil) specifically so the chunk size is not exceeded; this overflow defeats that intent. Content coverage and offset reconstruction are never broken — only the size bound.

Fix

Replace the fixed-width offset window with a token-count-aware greedy window that grows up to subchunks_per_chunk subchunks, then shrinks to stay within chunk_size; the stride never advances past the last included subchunk, so no subchunk is skipped (full content coverage preserved). It is identical to the previous behavior whenever no shrink is needed (the word/subword counters, where whitespace is 0 tokens, are unaffected). Applies to both the AI and non-AI overlap paths (identical code).

Tests

Adds test_overlap_never_exceeds_chunk_size to tests/test_semchunk.py. Verified via a 20k+-case fuzz over the char counter: 0 size violations, 0 reconstruction mismatches; non-overlap and word-counter overlap paths unchanged.

…okens

When overlap is enabled, output chunks are built by spanning several
subchunks by offset, which re-includes the whitespace separators between
them (stripped when the subchunks were formed). With a whitespace-
significant token counter (e.g. a character counter), those separators
count as tokens and can push a merged chunk past chunk_size, breaking the
documented maximum -- e.g. text='aa  bb  cc  dd', chunk_size=4, overlap=0.5
produced ['aa  bb', ...] at 6 tokens each.

Replace the fixed-width offset window with a token-count-aware greedy
window that shrinks to stay within chunk_size while still guaranteeing
full content coverage (stride never skips past the last included
subchunk). Identical to the previous behavior whenever no shrink is
needed. Applies to both the AI and non-AI overlap paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01N6RtoHuxrDqTUo9Mw9h4Cv
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.

1 participant