Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions mempalace/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 = (
Expand All @@ -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
Expand Down
18 changes: 12 additions & 6 deletions mempalace/miner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}")
Expand Down
81 changes: 70 additions & 11 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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():
Expand Down