diff --git a/mempalace/config.py b/mempalace/config.py index 29fcb340f9..1f805547c1 100644 --- a/mempalace/config.py +++ b/mempalace/config.py @@ -622,7 +622,8 @@ def _validated_chunk_config(self): Enforces the invariants the miner relies on: * ``chunk_size >= 1`` - * ``0 <= chunk_overlap < chunk_size`` — equality would loop forever + * ``0 <= chunk_overlap <= chunk_size // 2``. A larger overlap can + loop the miner forever on short-line content (#2056) * ``min_chunk_size <= chunk_size`` — otherwise no chunk is ever large enough to file, and ingest silently produces 0 drawers @@ -635,12 +636,14 @@ def _validated_chunk_config(self): "min_chunk_size", DEFAULT_MIN_CHUNK_SIZE, minimum=0 ) - if chunk_overlap >= chunk_size: - chunk_overlap = ( - DEFAULT_CHUNK_OVERLAP - if DEFAULT_CHUNK_OVERLAP < chunk_size - else max(0, chunk_size - 1) - ) + if chunk_overlap > chunk_size // 2: + # Overlap past half the chunk size can hang miner.chunk_text's + # windowing loop on short-line content (#2056): a boundary pull can + # shrink a chunk below chunk_overlap, so + # ``start = end - chunk_overlap`` stops advancing. Repair to the + # default when it is still at most half, else clamp to the largest + # safe overlap. + chunk_overlap = min(DEFAULT_CHUNK_OVERLAP, chunk_size // 2) if min_chunk_size > chunk_size: min_chunk_size = ( @@ -656,7 +659,7 @@ def chunk_size(self) -> int: @property def chunk_overlap(self) -> int: - """Overlap between adjacent chunks (validated, ``< chunk_size``).""" + """Overlap between adjacent chunks (validated, ``<= chunk_size // 2``).""" return self._validated_chunk_config()[1] @property diff --git a/mempalace/miner.py b/mempalace/miner.py index 3b286e5c65..6e46cda510 100644 --- a/mempalace/miner.py +++ b/mempalace/miner.py @@ -633,13 +633,19 @@ def chunk_text( raise ValueError(f"chunk_size must be a positive int, got {chunk_size!r}") if not isinstance(chunk_overlap, int) or chunk_overlap < 0: raise ValueError(f"chunk_overlap must be a non-negative int, got {chunk_overlap!r}") - if chunk_overlap >= chunk_size: - # ``start = end - chunk_overlap`` would not advance (or would go - # backward) when overlap >= size, producing an infinite loop on - # any non-empty input. + if chunk_overlap > chunk_size // 2: + # The windowing loop pulls ``end`` back to a boundary only when that + # boundary is past ``start + chunk_size // 2`` (the rfind guards below), + # so a pulled chunk always spans more than ``chunk_size // 2`` chars. + # ``start = end - chunk_overlap`` therefore advances only while + # ``chunk_overlap <= chunk_size // 2``; a larger overlap makes ``start`` + # stall or move backward and the loop spins forever on short-line + # content (#2056). The largest safe value is ``chunk_size // 2`` (half, + # rounded down for odd sizes), which still advances and stays allowed. raise ValueError( - f"chunk_overlap ({chunk_overlap}) must be less than chunk_size " - f"({chunk_size}); equality or greater would loop forever" + f"chunk_overlap ({chunk_overlap}) must be at most chunk_size // 2 " + f"({chunk_size // 2}); a larger overlap can loop forever on " + f"short-line content (#2056)" ) if not isinstance(min_chunk_size, int) or min_chunk_size < 0: raise ValueError(f"min_chunk_size must be a non-negative int, got {min_chunk_size!r}") diff --git a/tests/test_config.py b/tests/test_config.py index dc6486707a..dd9fdc3cb0 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -531,7 +531,7 @@ def test_iso_temporal_normalizes_plus_zero_offset_to_z(): # Backs the validated chunk_* properties added in #1024. Every property # resolves through ``_validated_chunk_config`` which (a) coerces to int # (or falls back to the documented default), (b) enforces the invariants -# ``chunk_text()`` needs (chunk_size >= 1, chunk_overlap < chunk_size, +# ``chunk_text()`` needs (chunk_size >= 1, chunk_overlap <= chunk_size // 2, # min_chunk_size <= chunk_size). A bad config.json must NEVER hang # ingest — repair, don't raise. @@ -593,23 +593,55 @@ def test_chunk_config_zero_chunk_size_falls_back(tmp_path): def test_chunk_config_overlap_at_or_above_size_repaired(tmp_path): - """``chunk_overlap >= chunk_size`` is the hang condition; repair to - the documented default when the default fits, otherwise to - ``chunk_size - 1``.""" + """``chunk_overlap`` above ``chunk_size // 2`` is the hang condition + (#2056); repair to the documented default when it stays at or below + half, otherwise clamp to ``chunk_size // 2``. Here the default fits.""" cfg = _write_config(tmp_path, chunk_size=900, chunk_overlap=900) assert cfg.chunk_size == 900 - # 100 (default) fits inside 900 → use the default. + # 100 (default) is at most 900 // 2, so use the default. assert cfg.chunk_overlap == 100 - assert cfg.chunk_overlap < cfg.chunk_size + assert cfg.chunk_overlap <= cfg.chunk_size // 2 def test_chunk_config_overlap_repair_when_default_doesnt_fit(tmp_path): - """Tiny chunk_size where the default overlap (100) wouldn't fit: - repair to ``chunk_size - 1`` instead.""" + """Tiny chunk_size where the default overlap (100) exceeds half the + chunk size: clamp to ``chunk_size // 2``, the largest safe overlap.""" cfg = _write_config(tmp_path, chunk_size=50, chunk_overlap=100) assert cfg.chunk_size == 50 - assert cfg.chunk_overlap == 49 # max(0, chunk_size - 1) - assert cfg.chunk_overlap < cfg.chunk_size + assert cfg.chunk_overlap == 25 # min(DEFAULT_CHUNK_OVERLAP, chunk_size // 2) + assert cfg.chunk_overlap <= cfg.chunk_size // 2 + + +def test_chunk_config_overlap_above_half_repaired(tmp_path): + """#2056: an overlap between ``chunk_size // 2`` and ``chunk_size`` used + to pass validation and could hang the miner on short-line content. It is + now repaired down to a safe value.""" + cfg = _write_config(tmp_path, chunk_size=100, chunk_overlap=80) + assert cfg.chunk_size == 100 + assert cfg.chunk_overlap == 50 # min(100, 100 // 2) + assert cfg.chunk_overlap <= cfg.chunk_size // 2 + + +def test_chunk_config_overlap_at_half_preserved(tmp_path): + """Exactly 50% overlap (``== chunk_size // 2``) is safe and must be kept + unchanged.""" + cfg = _write_config(tmp_path, chunk_size=800, chunk_overlap=400) + assert cfg.chunk_size == 800 + assert cfg.chunk_overlap == 400 + assert cfg.chunk_overlap <= cfg.chunk_size // 2 + + +def test_chunk_config_overlap_repair_odd_size_and_default_equals_half(tmp_path): + """Floor-boundary repairs (#2056): odd chunk_size floors ``// 2``, and the + case where DEFAULT_CHUNK_OVERLAP (100) equals chunk_size // 2 exactly.""" + # Odd size: 101 // 2 == 50, so an over-half overlap clamps to min(100, 50). + cfg = _write_config(tmp_path, chunk_size=101, chunk_overlap=100) + assert cfg.chunk_overlap == 50 + assert cfg.chunk_overlap <= cfg.chunk_size // 2 + # DEFAULT_CHUNK_OVERLAP (100) == 200 // 2: repair clamps to exactly 100. + cfg2 = _write_config(tmp_path, chunk_size=200, chunk_overlap=180) + assert cfg2.chunk_overlap == 100 + assert cfg2.chunk_overlap <= cfg2.chunk_size // 2 def test_chunk_config_min_chunk_size_above_size_repaired(tmp_path): @@ -710,13 +742,40 @@ def test_chunk_text_rejects_non_positive_chunk_size(): chunk_text("some content", "src.txt", chunk_size=-1) -def test_chunk_text_rejects_overlap_at_or_above_size(): +def test_chunk_text_rejects_overlap_above_half_size(): + """#2056: chunk_overlap > chunk_size // 2 can loop forever on short-line + content, so chunk_text now rejects it fast (not only overlap >= size).""" from mempalace.miner import chunk_text + # overlap >= chunk_size (the original #1024 guard) stays rejected. with pytest.raises(ValueError, match="chunk_overlap"): chunk_text("some content", "src.txt", chunk_size=100, chunk_overlap=100) with pytest.raises(ValueError, match="chunk_overlap"): chunk_text("some content", "src.txt", chunk_size=100, chunk_overlap=200) + # NEW: overlap strictly above half is now rejected too. + with pytest.raises(ValueError, match="chunk_overlap"): + chunk_text("some content", "src.txt", chunk_size=100, chunk_overlap=51) + with pytest.raises(ValueError, match="chunk_overlap"): + chunk_text("some content", "src.txt", chunk_size=50, chunk_overlap=49) + + +def test_chunk_text_overlap_boundary_at_half_size(): + """The exact safety boundary is ``chunk_size // 2``: overlap == half is + accepted and terminates; overlap == half + 1 is rejected because it can + loop forever on content whose lines are about half the chunk size (#2056). + """ + from mempalace.miner import chunk_text + + worst = ("x" * 10 + "\n") * 40 # 11-char lines = 20 // 2 + 1 + # overlap == chunk_size // 2 -> safe, returns a list (does not hang). + assert isinstance(chunk_text(worst, "src.txt", 20, 10), list) + # overlap == chunk_size // 2 + 1 -> rejected fast (would otherwise hang). + with pytest.raises(ValueError, match="chunk_overlap"): + chunk_text(worst, "src.txt", 20, 11) + # Odd chunk_size floors: 101 // 2 == 50, so 50 is accepted, 51 rejected. + assert isinstance(chunk_text("word " * 100, "src.txt", 101, 50), list) + with pytest.raises(ValueError, match="chunk_overlap"): + chunk_text("word " * 100, "src.txt", 101, 51) def test_chunk_text_rejects_negative_overlap():