Merge pull request #1554 from mvalentsev/fix/1455-max-chunks-configurable
fix(miner): configurable + raised MAX_CHUNKS_PER_FILE (#1455)
This commit is contained in:
Igor Lins e Silva
GitHub
commit
498b22ffed
|
|
@ -58,11 +58,7 @@ def build_corpus(dest: Path, n_files: int, paragraphs_per_file: int, seed: int)
|
|||
paragraphs.append(" ".join(words))
|
||||
(dest / f"doc_{i:03d}.md").write_text("\n\n".join(paragraphs))
|
||||
(dest / "mempalace.yaml").write_text(
|
||||
"wing: bench\n"
|
||||
"rooms:\n"
|
||||
" - name: general\n"
|
||||
" description: all\n"
|
||||
" keywords: [general]\n"
|
||||
"wing: bench\nrooms:\n - name: general\n description: all\n keywords: [general]\n"
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -121,8 +117,7 @@ def _process_file_unbatched(filepath, project_path, collection, wing, rooms, age
|
|||
]
|
||||
closet_lines = build_closet_lines(source_file, drawer_ids, content, wing, room)
|
||||
closet_id_base = (
|
||||
f"closet_{wing}_{room}_"
|
||||
f"{hashlib.sha256(source_file.encode()).hexdigest()[:24]}"
|
||||
f"closet_{wing}_{room}_{hashlib.sha256(source_file.encode()).hexdigest()[:24]}"
|
||||
)
|
||||
closet_meta = {
|
||||
"wing": wing,
|
||||
|
|
@ -155,7 +150,7 @@ def mine_once(project_dir: str, palace_path: str, batched: bool) -> tuple[int, f
|
|||
t0 = time.perf_counter()
|
||||
for filepath in files:
|
||||
if batched:
|
||||
drawers, _ = miner.process_file(
|
||||
drawers, _, _ = miner.process_file(
|
||||
filepath=filepath,
|
||||
project_path=project_path,
|
||||
collection=collection,
|
||||
|
|
@ -217,9 +212,9 @@ def run_scenario(label: str, n_files: int, paragraphs_per_file: int, seed: int)
|
|||
|
||||
|
||||
SCENARIOS = {
|
||||
"small": ("Small files (~50 paragraphs)", 10, 50),
|
||||
"small": ("Small files (~50 paragraphs)", 10, 50),
|
||||
"medium": ("Medium files (~200 paragraphs)", 20, 200),
|
||||
"large": ("Large files (~500 paragraphs)", 10, 500),
|
||||
"large": ("Large files (~500 paragraphs)", 10, 500),
|
||||
}
|
||||
|
||||
|
||||
|
|
@ -237,7 +232,9 @@ def _env_summary(device_label: str) -> list[str]:
|
|||
import onnxruntime as ort
|
||||
|
||||
ort_v = ort.__version__
|
||||
providers = ",".join(p.replace("ExecutionProvider", "") for p in ort.get_available_providers())
|
||||
providers = ",".join(
|
||||
p.replace("ExecutionProvider", "") for p in ort.get_available_providers()
|
||||
)
|
||||
except Exception:
|
||||
ort_v = "?"
|
||||
providers = "?"
|
||||
|
|
|
|||
|
|
@ -527,6 +527,7 @@ def cmd_mine(args):
|
|||
dry_run=args.dry_run,
|
||||
respect_gitignore=not args.no_gitignore,
|
||||
include_ignored=include_ignored,
|
||||
max_chunks_per_file=getattr(args, "max_chunks_per_file", None),
|
||||
)
|
||||
except MineAlreadyRunning as exc:
|
||||
# A live MCP server or another mine is already writing to this
|
||||
|
|
@ -1302,6 +1303,20 @@ def main():
|
|||
default="exchange",
|
||||
help="Extraction strategy for convos mode: 'exchange' (default) or 'general' (5 memory types)",
|
||||
)
|
||||
from . import miner as _miner_for_default
|
||||
|
||||
p_mine.add_argument(
|
||||
"--max-chunks-per-file",
|
||||
type=int,
|
||||
default=None,
|
||||
metavar="N",
|
||||
help=(
|
||||
f"Per-file chunk cap; files producing more chunks are skipped with a "
|
||||
f"summary counter. Default {_miner_for_default.MAX_CHUNKS_PER_FILE} "
|
||||
f"(or MEMPALACE_MAX_CHUNKS_PER_FILE). Set 0 to disable. Lower this on "
|
||||
f"Windows if you hit ONNX bad_alloc (#1455)."
|
||||
),
|
||||
)
|
||||
|
||||
# sweep
|
||||
p_sweep = sub.add_parser(
|
||||
|
|
|
|||
|
|
@ -83,13 +83,20 @@ from .config import ( # noqa: E402 (kept here for the legacy alias)
|
|||
|
||||
DRAWER_UPSERT_BATCH_SIZE = 1000
|
||||
MAX_FILE_SIZE = 500 * 1024 * 1024 # 500 MB — skip files larger than this.
|
||||
# A single file producing more chunks than this is almost always a generated
|
||||
# artifact (CSV/JSON dump, lockfile not in SKIP_FILENAMES, etc.). Embedding
|
||||
# thousands of chunks from one file in one batch has triggered ONNX runtime
|
||||
# `bad allocation` errors on Windows (#1296). The cap is conservative: a
|
||||
# 500-chunk file at CHUNK_SIZE=800 is ~400 KB of source, which covers most
|
||||
# legitimate hand-written content while bounding the worst-case batch.
|
||||
MAX_CHUNKS_PER_FILE = 500
|
||||
# A safety rail against pathological generated artifacts (lockfiles not in
|
||||
# SKIP_FILENAMES, vendored data dumps, etc.). Originally 500 to bound ONNX
|
||||
# runtime `bad allocation` errors on Windows (#1296), but at CHUNK_SIZE=800
|
||||
# that capped legitimate long-form content (#1455: full-text scholarly
|
||||
# editions, novels) at ~400 KB. The new default leaves two orders of
|
||||
# magnitude of safety margin against the original lockfile case
|
||||
# (~1124 chunks for `pnpm-lock.yaml` per #1296) while not touching
|
||||
# hand-written prose. Per-ONNX-call exposure is bounded by
|
||||
# `DRAWER_UPSERT_BATCH_SIZE` (1000 chunks/batch) regardless of this cap,
|
||||
# so the cap is a per-file admission rail, not a per-batch limit. Lower
|
||||
# this via `MEMPALACE_MAX_CHUNKS_PER_FILE` or
|
||||
# `mempalace mine --max-chunks-per-file N` if you hit ONNX bad_alloc on
|
||||
# Windows; set to 0 to disable the cap entirely.
|
||||
MAX_CHUNKS_PER_FILE = 50_000
|
||||
# Long Claude Code sessions and large transcript exports routinely exceed
|
||||
# 10 MB. The cap exists as a defensive rail against pathological binary
|
||||
# files, not as a limit on legitimate text. Per-drawer size is bounded
|
||||
|
|
@ -99,6 +106,48 @@ MAX_CHUNKS_PER_FILE = 500
|
|||
# memory before chunking), so memory use scales with source size too.
|
||||
|
||||
|
||||
def _resolve_max_chunks_per_file(override: Optional[int] = None) -> int:
|
||||
"""Resolve the effective per-file chunk cap.
|
||||
|
||||
Precedence: ``override`` (CLI flag) > ``MEMPALACE_MAX_CHUNKS_PER_FILE``
|
||||
env var > module-level ``MAX_CHUNKS_PER_FILE`` default. A sentinel
|
||||
value of ``0`` (from any source) disables the cap entirely. Negative
|
||||
values from either source emit a stderr warning and fall back to the
|
||||
module default so a misconfigured ``--max-chunks-per-file=-500`` typo
|
||||
(meaning "no, don't lower it that much") does not silently disable
|
||||
the cap and OOM on a generated artifact.
|
||||
"""
|
||||
if override is not None:
|
||||
if override < 0:
|
||||
print(
|
||||
f" ! WARNING: --max-chunks-per-file={override} is negative; "
|
||||
f"using default {MAX_CHUNKS_PER_FILE}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return MAX_CHUNKS_PER_FILE
|
||||
return int(override)
|
||||
raw = os.environ.get("MEMPALACE_MAX_CHUNKS_PER_FILE")
|
||||
if raw is None:
|
||||
return MAX_CHUNKS_PER_FILE
|
||||
try:
|
||||
val = int(raw)
|
||||
except ValueError:
|
||||
print(
|
||||
f" ! WARNING: MEMPALACE_MAX_CHUNKS_PER_FILE={raw!r} is not an integer; "
|
||||
f"using default {MAX_CHUNKS_PER_FILE}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return MAX_CHUNKS_PER_FILE
|
||||
if val < 0:
|
||||
print(
|
||||
f" ! WARNING: MEMPALACE_MAX_CHUNKS_PER_FILE={val} is negative; "
|
||||
f"using default {MAX_CHUNKS_PER_FILE}",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return MAX_CHUNKS_PER_FILE
|
||||
return val
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# IGNORE MATCHING
|
||||
# =============================================================================
|
||||
|
|
@ -887,23 +936,32 @@ def process_file(
|
|||
chunk_size: int = None,
|
||||
chunk_overlap: int = None,
|
||||
min_chunk_size: int = None,
|
||||
max_chunks_per_file: Optional[int] = None,
|
||||
) -> tuple:
|
||||
"""Read, chunk, route, and file one file. Returns (drawer_count, room_name)."""
|
||||
"""Read, chunk, route, and file one file.
|
||||
|
||||
Returns ``(drawer_count, room_name, skip_reason)``. ``skip_reason`` is
|
||||
``None`` on success and on every non-chunk-cap skip path: already
|
||||
filed (pre- or post-lock re-check), unreadable (``OSError``), or
|
||||
too-short content (below ``min_chunk_size``). It is ``"chunk_cap"``
|
||||
when the per-file chunk cap aborted the file. Callers use the tag to
|
||||
surface a separate counter in the mine summary (see #1455).
|
||||
"""
|
||||
effective_min = min_chunk_size if min_chunk_size is not None else MIN_CHUNK_SIZE
|
||||
|
||||
# Skip if already filed
|
||||
source_file = str(filepath)
|
||||
if not dry_run and file_already_mined(collection, source_file, check_mtime=True):
|
||||
return 0, "general"
|
||||
return 0, "general", None
|
||||
|
||||
try:
|
||||
content = filepath.read_text(encoding="utf-8", errors="replace")
|
||||
except OSError:
|
||||
return 0, "general"
|
||||
return 0, "general", None
|
||||
|
||||
content = content.strip()
|
||||
if len(content) < effective_min:
|
||||
return 0, "general"
|
||||
return 0, "general", None
|
||||
|
||||
room = detect_room(filepath, content, rooms, project_path)
|
||||
chunks = chunk_text(
|
||||
|
|
@ -914,16 +972,24 @@ def process_file(
|
|||
min_chunk_size=min_chunk_size,
|
||||
)
|
||||
|
||||
if len(chunks) > MAX_CHUNKS_PER_FILE:
|
||||
effective_cap = _resolve_max_chunks_per_file(max_chunks_per_file)
|
||||
if effective_cap > 0 and len(chunks) > effective_cap:
|
||||
# Skip notice goes to stderr alongside the existing symlink-skip
|
||||
# warning style (see ``scan_project``'s ``SKIP: <rel> (symlink)``
|
||||
# line). This keeps ``mempalace mine ... > out.log 2> err.log``
|
||||
# piping coherent: degraded outcomes on stderr, progress on stdout.
|
||||
print(
|
||||
f" ! [skip] {filepath.name[:50]:50} produced {len(chunks)} chunks "
|
||||
f"(> {MAX_CHUNKS_PER_FILE}); add to SKIP_FILENAMES or .gitignore"
|
||||
f"(> {effective_cap}); raise via --max-chunks-per-file or "
|
||||
f"MEMPALACE_MAX_CHUNKS_PER_FILE (set 0 to disable), or add to "
|
||||
f"SKIP_FILENAMES if this is a generated artifact",
|
||||
file=sys.stderr,
|
||||
)
|
||||
return 0, room
|
||||
return 0, room, "chunk_cap"
|
||||
|
||||
if dry_run:
|
||||
print(f" [DRY RUN] {filepath.name} -> room:{room} ({len(chunks)} drawers)")
|
||||
return len(chunks), room
|
||||
return len(chunks), room, None
|
||||
|
||||
# Lock this file so concurrent agents don't interleave delete+insert.
|
||||
# Without the lock, two agents can both pass file_already_mined(),
|
||||
|
|
@ -931,7 +997,7 @@ def process_file(
|
|||
with mine_lock(source_file):
|
||||
# Re-check after acquiring lock — another agent may have just finished
|
||||
if file_already_mined(collection, source_file, check_mtime=True):
|
||||
return 0, room
|
||||
return 0, room, None
|
||||
|
||||
# Purge stale drawers for this file before re-inserting the fresh chunks.
|
||||
# Converts modified-file re-mines from upsert-over-existing-IDs (which hits
|
||||
|
|
@ -1005,7 +1071,7 @@ def process_file(
|
|||
purge_file_closets(closets_col, source_file)
|
||||
upsert_closet_lines(closets_col, closet_id_base, closet_lines, closet_meta)
|
||||
|
||||
return drawers_added, room
|
||||
return drawers_added, room, None
|
||||
|
||||
|
||||
# =============================================================================
|
||||
|
|
@ -1102,6 +1168,7 @@ def mine(
|
|||
respect_gitignore: bool = True,
|
||||
include_ignored: list = None,
|
||||
files: list = None,
|
||||
max_chunks_per_file: Optional[int] = None,
|
||||
):
|
||||
"""Mine a project directory into the palace.
|
||||
|
||||
|
|
@ -1110,6 +1177,11 @@ def mine(
|
|||
caller (e.g. ``init`` showing a file-count estimate before the mine
|
||||
prompt) avoids walking the tree twice. When ``None`` (the default),
|
||||
``mine`` walks the tree itself just like before.
|
||||
|
||||
``max_chunks_per_file`` overrides the per-file chunk cap (see
|
||||
:func:`_resolve_max_chunks_per_file`). ``None`` defers to
|
||||
``MEMPALACE_MAX_CHUNKS_PER_FILE`` or ``MAX_CHUNKS_PER_FILE``; ``0``
|
||||
disables the cap entirely (#1455).
|
||||
"""
|
||||
if dry_run:
|
||||
return _mine_impl(
|
||||
|
|
@ -1122,6 +1194,7 @@ def mine(
|
|||
respect_gitignore=respect_gitignore,
|
||||
include_ignored=include_ignored,
|
||||
files=files,
|
||||
max_chunks_per_file=max_chunks_per_file,
|
||||
)
|
||||
|
||||
# MineAlreadyRunning propagates so the CLI can render a clear holder-aware
|
||||
|
|
@ -1138,6 +1211,7 @@ def mine(
|
|||
respect_gitignore=respect_gitignore,
|
||||
include_ignored=include_ignored,
|
||||
files=files,
|
||||
max_chunks_per_file=max_chunks_per_file,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -1151,6 +1225,7 @@ def _mine_impl(
|
|||
respect_gitignore: bool = True,
|
||||
include_ignored: list = None,
|
||||
files: list = None,
|
||||
max_chunks_per_file: Optional[int] = None,
|
||||
):
|
||||
from .config import MempalaceConfig
|
||||
|
||||
|
|
@ -1201,14 +1276,16 @@ def _mine_impl(
|
|||
|
||||
total_drawers = 0
|
||||
files_skipped = 0
|
||||
files_skipped_chunk_cap = 0
|
||||
files_processed = 0
|
||||
last_file = None
|
||||
room_counts = defaultdict(int)
|
||||
effective_chunk_cap = _resolve_max_chunks_per_file(max_chunks_per_file)
|
||||
|
||||
try:
|
||||
for i, filepath in enumerate(files, 1):
|
||||
try:
|
||||
drawers, room = process_file(
|
||||
drawers, room, skip_reason = process_file(
|
||||
filepath=filepath,
|
||||
project_path=project_path,
|
||||
collection=collection,
|
||||
|
|
@ -1220,6 +1297,11 @@ def _mine_impl(
|
|||
chunk_size=cfg_chunk_size,
|
||||
chunk_overlap=cfg_chunk_overlap,
|
||||
min_chunk_size=cfg_min_chunk_size,
|
||||
# Pass the already-resolved int so ``process_file``'s
|
||||
# ``override is not None`` branch skips the env re-read;
|
||||
# otherwise a malformed env var would emit its warning
|
||||
# per file.
|
||||
max_chunks_per_file=effective_chunk_cap,
|
||||
)
|
||||
except KeyboardInterrupt:
|
||||
# Re-raise so the outer handler prints the summary; we
|
||||
|
|
@ -1228,8 +1310,15 @@ def _mine_impl(
|
|||
raise
|
||||
files_processed = i
|
||||
last_file = filepath.name
|
||||
if drawers == 0 and not dry_run:
|
||||
# All zero-drawer outcomes increment ``files_skipped`` in both
|
||||
# modes so the summary "Files processed" arithmetic and the
|
||||
# residual-skip counter stay honest under ``--dry-run`` too. The
|
||||
# chunk-cap counter is partitioned out for its dedicated
|
||||
# summary line (see #1455 + Gemini review on PR #1554).
|
||||
if drawers == 0:
|
||||
files_skipped += 1
|
||||
if skip_reason == "chunk_cap":
|
||||
files_skipped_chunk_cap += 1
|
||||
else:
|
||||
total_drawers += drawers
|
||||
room_counts[room] += 1
|
||||
|
|
@ -1255,7 +1344,26 @@ def _mine_impl(
|
|||
print(f"\n{'=' * 55}")
|
||||
print(" Done.")
|
||||
print(f" Files processed: {len(files) - files_skipped}")
|
||||
print(f" Files skipped (already filed): {files_skipped}")
|
||||
# The residual skip bucket label depends on mode: dry-run bypasses
|
||||
# the already-mined check, so the only paths producing (0, room,
|
||||
# None) under dry_run are OSError / too-short / post-lock re-check
|
||||
# (and re-check itself is unreachable when nothing is being
|
||||
# written). Outside dry_run, the dominant case is "already filed".
|
||||
residual_label = (
|
||||
"Files skipped (read error or too short)"
|
||||
if dry_run
|
||||
else "Files skipped (already filed or other)"
|
||||
)
|
||||
print(f" {residual_label}: {max(0, files_skipped - files_skipped_chunk_cap)}")
|
||||
if files_skipped_chunk_cap > 0:
|
||||
# ``effective_chunk_cap`` is necessarily > 0 here: ``process_file``
|
||||
# only emits the ``"chunk_cap"`` skip_reason when its own
|
||||
# ``effective_cap > 0`` guard passes (see ``process_file``).
|
||||
print(
|
||||
f" Files skipped (chunk cap {effective_chunk_cap}): {files_skipped_chunk_cap} "
|
||||
f"(raise via --max-chunks-per-file or MEMPALACE_MAX_CHUNKS_PER_FILE; "
|
||||
f"set 0 to disable)"
|
||||
)
|
||||
print(f" Drawers filed: {total_drawers}")
|
||||
print("\n By room:")
|
||||
for room, count in sorted(room_counts.items(), key=lambda x: x[1], reverse=True):
|
||||
|
|
|
|||
|
|
@ -566,6 +566,7 @@ def test_cmd_mine_projects_mode(mock_config_cls):
|
|||
dry_run=False,
|
||||
respect_gitignore=True,
|
||||
include_ignored=[],
|
||||
max_chunks_per_file=None,
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -642,7 +642,7 @@ def test_process_file_uses_bounded_upsert_batches(tmp_path, monkeypatch):
|
|||
monkeypatch.setattr(miner, "detect_hall", lambda content: "code")
|
||||
monkeypatch.setattr(miner, "_extract_entities_for_metadata", lambda content: "")
|
||||
|
||||
drawers, room = miner.process_file(
|
||||
drawers, room, skip_reason = miner.process_file(
|
||||
source,
|
||||
tmp_path,
|
||||
col,
|
||||
|
|
@ -654,6 +654,7 @@ def test_process_file_uses_bounded_upsert_batches(tmp_path, monkeypatch):
|
|||
|
||||
assert drawers == 5
|
||||
assert room == "general"
|
||||
assert skip_reason is None
|
||||
assert col.batch_sizes == [2, 2, 1]
|
||||
|
||||
|
||||
|
|
@ -969,7 +970,7 @@ def test_mine_keyboard_interrupt_prints_summary_and_exits_130(tmp_path, capsys):
|
|||
call_count["n"] += 1
|
||||
if call_count["n"] == 2:
|
||||
raise KeyboardInterrupt
|
||||
return (1, "general")
|
||||
return (1, "general", None)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
with pytest.raises(SystemExit) as exc_info:
|
||||
|
|
@ -1022,16 +1023,18 @@ def test_skip_filenames_includes_lockfiles():
|
|||
assert "yarn.lock" in miner.SKIP_FILENAMES
|
||||
|
||||
|
||||
def test_process_file_skips_when_chunks_exceed_max(tmp_path, monkeypatch):
|
||||
"""A file producing more than MAX_CHUNKS_PER_FILE chunks must be skipped
|
||||
with a clear message and zero upserts. Generated artifacts (CSVs, lock
|
||||
files not in SKIP_FILENAMES) hit this — the cap is what prevents ONNX
|
||||
bad_alloc on Windows when the embedder is asked to swallow thousands of
|
||||
chunks in one batch (#1296)."""
|
||||
def test_process_file_skips_when_chunks_exceed_max(tmp_path, monkeypatch, capsys):
|
||||
"""A file exceeding the per-file chunk cap is skipped with a tagged
|
||||
return and a stderr/stdout message pointing at the config override. The
|
||||
cap is the rail against pathological artifacts (CSVs, lockfiles not in
|
||||
SKIP_FILENAMES) and against ONNX bad_alloc on Windows (#1296); #1455
|
||||
raised the default and made the cap configurable so legitimate
|
||||
long-form content is not silently dropped."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
monkeypatch.setattr(miner, "MAX_CHUNKS_PER_FILE", 5)
|
||||
over_cap = [{"content": f"chunk {i}", "chunk_index": i} for i in range(7)]
|
||||
monkeypatch.setattr(miner, "chunk_text", lambda content, source_file, **kwargs: over_cap)
|
||||
|
|
@ -1041,7 +1044,7 @@ def test_process_file_skips_when_chunks_exceed_max(tmp_path, monkeypatch):
|
|||
col = MagicMock()
|
||||
col.get.return_value = {"ids": []}
|
||||
|
||||
drawers, room = miner.process_file(
|
||||
drawers, room, skip_reason = miner.process_file(
|
||||
source,
|
||||
tmp_path,
|
||||
col,
|
||||
|
|
@ -1052,7 +1055,274 @@ def test_process_file_skips_when_chunks_exceed_max(tmp_path, monkeypatch):
|
|||
)
|
||||
|
||||
assert drawers == 0
|
||||
assert skip_reason == "chunk_cap"
|
||||
col.upsert.assert_not_called()
|
||||
captured = capsys.readouterr()
|
||||
# Skip notice goes to stderr to match the existing symlink-skip
|
||||
# convention in ``scan_project`` so log piping stays coherent.
|
||||
assert "[skip]" in captured.err
|
||||
assert "--max-chunks-per-file" in captured.err
|
||||
assert "MEMPALACE_MAX_CHUNKS_PER_FILE" in captured.err
|
||||
assert "[skip]" not in captured.out
|
||||
|
||||
|
||||
def test_resolve_max_chunks_default_when_no_override_no_env(monkeypatch):
|
||||
"""With no override and no env var, the module-level default applies."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
assert miner._resolve_max_chunks_per_file(None) == miner.MAX_CHUNKS_PER_FILE
|
||||
|
||||
|
||||
def test_resolve_max_chunks_env_var_wins_over_default(monkeypatch):
|
||||
"""A numeric env var overrides the module default."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.setenv("MEMPALACE_MAX_CHUNKS_PER_FILE", "777")
|
||||
assert miner._resolve_max_chunks_per_file(None) == 777
|
||||
|
||||
|
||||
def test_resolve_max_chunks_override_wins_over_env(monkeypatch):
|
||||
"""An explicit override (CLI flag plumbed in) wins over the env var."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.setenv("MEMPALACE_MAX_CHUNKS_PER_FILE", "777")
|
||||
assert miner._resolve_max_chunks_per_file(123) == 123
|
||||
|
||||
|
||||
def test_resolve_max_chunks_sentinel_zero_disables(monkeypatch):
|
||||
"""Sentinel ``0`` from override or env means "no cap"."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
assert miner._resolve_max_chunks_per_file(0) == 0
|
||||
|
||||
monkeypatch.setenv("MEMPALACE_MAX_CHUNKS_PER_FILE", "0")
|
||||
assert miner._resolve_max_chunks_per_file(None) == 0
|
||||
|
||||
|
||||
def test_resolve_max_chunks_invalid_env_falls_back_to_default(monkeypatch, capsys):
|
||||
"""A non-integer env value warns and uses the module default. This
|
||||
keeps a misconfigured shell from silently dropping content."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.setenv("MEMPALACE_MAX_CHUNKS_PER_FILE", "banana")
|
||||
assert miner._resolve_max_chunks_per_file(None) == miner.MAX_CHUNKS_PER_FILE
|
||||
err = capsys.readouterr().err
|
||||
assert "MEMPALACE_MAX_CHUNKS_PER_FILE" in err
|
||||
assert "banana" in err
|
||||
|
||||
|
||||
def test_resolve_max_chunks_negative_env_falls_back_to_default(monkeypatch, capsys):
|
||||
"""A negative env value warns and uses the module default."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.setenv("MEMPALACE_MAX_CHUNKS_PER_FILE", "-5")
|
||||
assert miner._resolve_max_chunks_per_file(None) == miner.MAX_CHUNKS_PER_FILE
|
||||
err = capsys.readouterr().err
|
||||
assert "MEMPALACE_MAX_CHUNKS_PER_FILE" in err
|
||||
assert "-5" in err
|
||||
|
||||
|
||||
def test_process_file_sentinel_zero_disables_cap(tmp_path, monkeypatch):
|
||||
"""With ``max_chunks_per_file=0`` even a pathologically large chunk
|
||||
count is processed (no skip)."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
monkeypatch.setattr(miner, "MAX_CHUNKS_PER_FILE", 5)
|
||||
big = [{"content": f"chunk {i}", "chunk_index": i} for i in range(20)]
|
||||
monkeypatch.setattr(miner, "chunk_text", lambda content, source_file, **kwargs: big)
|
||||
monkeypatch.setattr(miner, "detect_room", lambda *a, **k: "general")
|
||||
monkeypatch.setattr(miner, "_extract_entities_for_metadata", lambda content: "")
|
||||
monkeypatch.setattr(miner, "build_closet_lines", lambda *a, **k: [])
|
||||
monkeypatch.setattr(miner, "purge_file_closets", lambda *a, **k: None)
|
||||
monkeypatch.setattr(miner, "upsert_closet_lines", lambda *a, **k: None)
|
||||
|
||||
source = tmp_path / "huge.csv"
|
||||
source.write_text("payload\n" * 500, encoding="utf-8")
|
||||
col = MagicMock()
|
||||
col.get.return_value = {"ids": []}
|
||||
|
||||
drawers, _room, skip_reason = miner.process_file(
|
||||
source,
|
||||
tmp_path,
|
||||
col,
|
||||
"wing",
|
||||
[{"name": "general", "description": "General"}],
|
||||
"agent",
|
||||
False,
|
||||
max_chunks_per_file=0,
|
||||
)
|
||||
|
||||
assert drawers == 20
|
||||
assert skip_reason is None
|
||||
col.upsert.assert_called()
|
||||
|
||||
|
||||
def test_mine_summary_separates_chunk_cap_skips(tmp_path, monkeypatch, capsys):
|
||||
"""Summary distinguishes residual skips from "chunk cap" skips so a
|
||||
user can see immediately that legitimate content was dropped (#1455).
|
||||
The chunk-cap summary line appears only when count > 0."""
|
||||
from unittest.mock import patch
|
||||
|
||||
project_root = tmp_path / "proj"
|
||||
project_root.mkdir()
|
||||
_make_minable_project(project_root, n_files=3)
|
||||
palace_path = project_root / "palace"
|
||||
|
||||
seq = iter(
|
||||
[
|
||||
(5, "general", None),
|
||||
(0, "general", "chunk_cap"),
|
||||
(0, "general", None),
|
||||
]
|
||||
)
|
||||
|
||||
def fake_process_file(*args, **kwargs):
|
||||
return next(seq)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
mine(str(project_root), str(palace_path))
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "Files skipped (already filed or other): 1" in out
|
||||
assert "Files skipped (chunk cap" in out
|
||||
assert "--max-chunks-per-file" in out
|
||||
|
||||
|
||||
def test_mine_summary_omits_chunk_cap_line_when_zero(tmp_path, monkeypatch, capsys):
|
||||
"""When no file hits the chunk cap, the chunk-cap summary line is not
|
||||
printed at all, which keeps the happy-path output unchanged."""
|
||||
from unittest.mock import patch
|
||||
|
||||
project_root = tmp_path / "proj"
|
||||
project_root.mkdir()
|
||||
_make_minable_project(project_root, n_files=2)
|
||||
palace_path = project_root / "palace"
|
||||
|
||||
def fake_process_file(*args, **kwargs):
|
||||
return (3, "general", None)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
mine(str(project_root), str(palace_path))
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "Files skipped (already filed or other): 0" in out
|
||||
assert "chunk cap" not in out
|
||||
|
||||
|
||||
def test_resolve_max_chunks_negative_override_falls_back_to_default(monkeypatch, capsys):
|
||||
"""A negative CLI override warns and falls back to the module default.
|
||||
Symmetric with the env-var path so ``--max-chunks-per-file=-500`` (a
|
||||
typo meaning "no, don't lower it that much") does not silently
|
||||
disable the cap and OOM the embedder on the original lockfile."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
assert miner._resolve_max_chunks_per_file(-5) == miner.MAX_CHUNKS_PER_FILE
|
||||
err = capsys.readouterr().err
|
||||
assert "--max-chunks-per-file" in err
|
||||
assert "-5" in err
|
||||
|
||||
|
||||
def test_resolve_max_chunks_reads_module_attribute_at_call_time(monkeypatch):
|
||||
"""The resolver reads ``miner.MAX_CHUNKS_PER_FILE`` lazily, so a
|
||||
monkeypatch landed at test setup is honored. Regression guard against
|
||||
a future refactor that captures the import-time default."""
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
monkeypatch.setattr(miner, "MAX_CHUNKS_PER_FILE", 7)
|
||||
assert miner._resolve_max_chunks_per_file(None) == 7
|
||||
|
||||
|
||||
def test_process_file_chunk_cap_under_dry_run(tmp_path, monkeypatch):
|
||||
"""Dry-run is the natural audit path for #1455; chunk-cap drops must
|
||||
return a tagged skip_reason there too so the summary counter can fire."""
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from mempalace import miner
|
||||
|
||||
monkeypatch.delenv("MEMPALACE_MAX_CHUNKS_PER_FILE", raising=False)
|
||||
monkeypatch.setattr(miner, "MAX_CHUNKS_PER_FILE", 5)
|
||||
over_cap = [{"content": f"chunk {i}", "chunk_index": i} for i in range(7)]
|
||||
monkeypatch.setattr(miner, "chunk_text", lambda content, source_file, **kwargs: over_cap)
|
||||
|
||||
source = tmp_path / "big.csv"
|
||||
source.write_text("payload\n" * 200, encoding="utf-8")
|
||||
col = MagicMock()
|
||||
col.get.return_value = {"ids": []}
|
||||
|
||||
drawers, _room, skip_reason = miner.process_file(
|
||||
source,
|
||||
tmp_path,
|
||||
col,
|
||||
"wing",
|
||||
[{"name": "general", "description": "General"}],
|
||||
"agent",
|
||||
True, # dry_run
|
||||
)
|
||||
|
||||
assert drawers == 0
|
||||
assert skip_reason == "chunk_cap"
|
||||
|
||||
|
||||
def test_mine_dry_run_summary_counts_chunk_cap_drops(tmp_path, capsys):
|
||||
"""The summary under dry-run also splits out chunk-cap skips. Without
|
||||
this a reporter running ``--dry-run`` to validate the new default
|
||||
against their corpus would see "Files processed: N / Files skipped: 0"
|
||||
even when chunk-cap drops occurred, which is exactly the silent-drop
|
||||
UX bug that #1455 is fixing."""
|
||||
from unittest.mock import patch
|
||||
|
||||
project_root = tmp_path / "proj"
|
||||
project_root.mkdir()
|
||||
_make_minable_project(project_root, n_files=3)
|
||||
palace_path = project_root / "palace"
|
||||
|
||||
seq = iter(
|
||||
[
|
||||
(5, "general", None),
|
||||
(0, "general", "chunk_cap"),
|
||||
(4, "general", None),
|
||||
]
|
||||
)
|
||||
|
||||
def fake_process_file(*args, **kwargs):
|
||||
return next(seq)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
mine(str(project_root), str(palace_path), dry_run=True)
|
||||
|
||||
out = capsys.readouterr().out
|
||||
assert "Files skipped (chunk cap" in out
|
||||
assert "1 (raise via" in out
|
||||
|
||||
|
||||
def test_mine_plumbs_max_chunks_per_file_to_process_file(tmp_path):
|
||||
"""``mine(max_chunks_per_file=0)`` reaches ``process_file`` as kwarg=0,
|
||||
enabling the sentinel-disable path end-to-end. Guards the wiring
|
||||
``cmd_mine -> mine -> _mine_impl -> process_file``."""
|
||||
from unittest.mock import patch
|
||||
|
||||
project_root = tmp_path / "proj"
|
||||
project_root.mkdir()
|
||||
_make_minable_project(project_root, n_files=1)
|
||||
palace_path = project_root / "palace"
|
||||
|
||||
captured = {}
|
||||
|
||||
def fake_process_file(*args, **kwargs):
|
||||
captured["max_chunks_per_file"] = kwargs.get("max_chunks_per_file")
|
||||
return (1, "general", None)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
mine(str(project_root), str(palace_path), max_chunks_per_file=0)
|
||||
|
||||
assert captured["max_chunks_per_file"] == 0
|
||||
|
||||
|
||||
def test_mine_arbitrary_exception_prints_summary_and_reraises(tmp_path, capsys):
|
||||
|
|
@ -1074,7 +1344,7 @@ def test_mine_arbitrary_exception_prints_summary_and_reraises(tmp_path, capsys):
|
|||
call_count["n"] += 1
|
||||
if call_count["n"] == 2:
|
||||
raise RuntimeError("simulated ONNX bad_alloc")
|
||||
return (1, "general")
|
||||
return (1, "general", None)
|
||||
|
||||
with patch("mempalace.miner.process_file", side_effect=fake_process_file):
|
||||
with pytest.raises(RuntimeError, match="simulated ONNX bad_alloc"):
|
||||
|
|
|
|||
Loading…
Reference in New Issue