fix(miner): reject chunk_overlap above half the chunk size to stop chunk_text hang (#2056) - #2058
Merged
igorls merged 1 commit intoJul 22, 2026
Conversation
…unk_text hang (MemPalace#2056) chunk_text's windowing loop (start = end - chunk_overlap) stops advancing when chunk_overlap exceeds chunk_size // 2 on short-line content, looping forever at 100% CPU. A paragraph or line boundary pull only moves end past start + chunk_size // 2, so a pulled chunk spans more than half the chunk size; the step then advances only while chunk_overlap <= chunk_size // 2. Tighten the existing guard from chunk_overlap >= chunk_size to chunk_overlap > chunk_size // 2 in both chunk_text (raise, with the numeric bound in the message) and MempalaceConfig._validated_chunk_config (repair to min(DEFAULT_CHUNK_OVERLAP, chunk_size // 2)). The windowing loop is unchanged, so every already-valid config produces byte-identical chunks; 50% overlap (chunk_overlap == chunk_size // 2) stays valid. Fixes MemPalace#2056.
mvalentsev
force-pushed
the
fix/2056-chunk-text-windowing-loop
branch
from
July 21, 2026 19:34
a5a5c99 to
bfd1e48
Compare
mvalentsev
marked this pull request as ready for review
July 21, 2026 19:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
chunk_text(mempalace/miner.py) can loop forever at 100% CPU whenchunk_overlapis set above half the chunk size and the content has short lines. This tightens the existing overlap guard, in bothchunk_textandMempalaceConfig._validated_chunk_config, fromchunk_overlap >= chunk_sizetochunk_overlap > chunk_size // 2, the exact point past which the windowing loop can stop advancing.Fixes #2056.
Root cause. The windowing loop advances with
start = end - chunk_overlap. A paragraph or line boundary pull only movesendback when the boundary sits paststart + chunk_size // 2, so a pulled chunk always spans more thanchunk_size // 2characters. The step therefore advances only whilechunk_overlap <= chunk_size // 2; a larger overlap makesstartstall or move backward, and on short-line content the loop oscillates over the same positions and never returns. The existing guards were added to stop exactly this loop (the config docstring and thechunk_texterror message both say such an overlap "would loop forever"), but they only rejectedchunk_overlap >= chunk_size, so anychunk_size // 2 < chunk_overlap < chunk_sizeslipped through.The boundary is exactly
chunk_size // 2.chunk_overlap <= chunk_size // 2always terminates: after a pullend - start > chunk_size // 2 >= chunk_overlap, sostartadvances by at least one. Confirmed for everychunk_sizein 4..400.chunk_overlap == chunk_size // 2 + 1can hang: content whose lines are about half the chunk size makes each pulled chunk span exactlychunk_size // 2 + 1, sostartdoes not move. Confirmed for everychunk_sizein 4..400, and on the realchunk_text(chunk_size=20, chunk_overlap=11hangs;20/10returns).So 50% overlap (
chunk_overlap == chunk_size // 2) stays valid; only strictly more than half is rejected, which narrows no useful configuration.Fix.
chunk_text(a public function): raiseValueErrorforchunk_overlap > chunk_size // 2, with the numeric bound in the message.MempalaceConfig._validated_chunk_config: aconfig.jsonoverlap above half is repaired tomin(DEFAULT_CHUNK_OVERLAP, chunk_size // 2), which is never above half. This matches the file's existing rule that a badconfig.jsonrepairs rather than raises.The windowing loop itself is untouched, so every already-valid config produces byte-identical chunks.
Behavior change. Direct
chunk_textcallers now get a fastValueErrorforchunk_size // 2 < chunk_overlap < chunk_size, where before they got a hang (or, on sparse content, a result). This also covers the module default overlap (100) when a caller overrideschunk_sizebelow 200 without passing an explicit overlap; the old guard already raised the same way forchunk_size <= 100. CLI and MCP users are unaffected: both mining paths read the validatedchunk_overlapproperty, which clamps an out-of-range overlap down tochunk_size // 2rather than raising. The default800 / 100config, and any overlap up to half the chunk size, are unchanged.This follows #2054 / #2055 (the O(N*K) line-locator cost), which flagged this loop as a separate bug. The partial guard came from #1024.
How to test
Reproduce on
develop, then confirm it is fixed here. Purechunk_text, no palace needed:Through the
mempalacebinary: set"chunk_size": 120, "chunk_overlap": 61in a palaceconfig.jsonandmempalace minea file whose lines are short. Ondevelopthe mine sits at 100% CPU and never completes; with this change it finishes (the overlap is repaired to 60).Automated:
The full suite is green apart from two pre-existing concurrency-lock timing tests (
test_writer_blocks_during_mine,test_mine_convos_refuses_concurrent_run_against_same_palace) that flake ondevelopas well and never touch chunking.Checklist
python -m pytest tests/ -v)ruff check .)