From 552239dcddca9631578f4011dd306e6ba3dc0155 Mon Sep 17 00:00:00 2001 From: jp Date: Sat, 23 May 2026 12:35:53 -0700 Subject: [PATCH 1/2] feat(miner): symbol_header_prefix kwarg in chunk_text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Optional keyword-only callable ``symbol_header_prefix(chunk_text, source_file, chunk_index) -> str``. When supplied, the returned header is prepended to each chunk with a blank-line separator before storage. Default ``None`` preserves current behavior exactly — all existing callers are unaffected. Discussed in #1384 — gives AST-lite symbol-header enrichment (and other representation-axis experiments like adaptmem's FT-Code + header stacking) a clean seam without forking ``chunk_text``. Chunk content gets ~50–200 chars longer when a header fires; the caller controls the budget. Existing miner test suite (tests/test_miner.py, 96 cases) passes unchanged on the techempower-org fork where the kwarg was first shipped. Co-Authored-By: Claude Opus 4.7 --- mempalace/miner.py | 38 ++++++++++++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/mempalace/miner.py b/mempalace/miner.py index aed181070d..4b79091c45 100644 --- a/mempalace/miner.py +++ b/mempalace/miner.py @@ -477,19 +477,37 @@ def chunk_text( chunk_size: int = None, chunk_overlap: int = None, min_chunk_size: int = None, + *, + symbol_header_prefix=None, ) -> list: """ Split content into drawer-sized chunks. Tries to split on paragraph/line boundaries. - Returns list of {"content": str, "chunk_index": int, "line_start": int, "line_end": int} - ``line_start`` / ``line_end`` are 1-indexed line numbers in the stripped - source, giving an approximate locator for where the chunk came from. - Closet pointers (Tier 6a) use this to emit ``YYYY-MM-DD:L42-L78`` segments - so retrieval can jump straight to the right span without opening the - whole drawer. - - Optional params override module-level defaults when provided. + Args: + content: text to chunk. + source_file: file path used for room/topic inference and (when + ``symbol_header_prefix`` is supplied) chunk enrichment. + chunk_size, chunk_overlap, min_chunk_size: optional overrides for + the module-level defaults. When ``None`` (default), the + module constants ``CHUNK_SIZE`` / ``CHUNK_OVERLAP`` / + ``MIN_CHUNK_SIZE`` are used. Validated as positive ints; an + overlap >= size is rejected to avoid an infinite loop. + symbol_header_prefix: optional callable + ``(chunk_text, source_file, chunk_index) -> str``. When + supplied, the returned header is prepended to each chunk + with a blank line separator before storage. Lets AST-lite + symbol enrichment (function names, class paths, imports) + and similar representation-axis experiments stack on this + code path without forking it. Default ``None`` preserves + original behavior exactly. Discussed in + MemPalace/mempalace#1384. + + Returns: + list of ``{"content": str, "chunk_index": int, "line_start": int, + "line_end": int}``. ``line_start`` / ``line_end`` are 1-indexed + line numbers in the stripped source, giving an approximate locator + for where the chunk came from. """ if chunk_size is None: chunk_size = CHUNK_SIZE @@ -543,6 +561,10 @@ def chunk_text( chunk = content[start:end].strip() if len(chunk) >= min_chunk_size: + if symbol_header_prefix is not None: + header = symbol_header_prefix(chunk, source_file, chunk_index) + if header: + chunk = f"{header}\n\n{chunk}" # Tier 6a — 1-indexed line range in the stripped source. # Approximate locator (±1 at boundaries is fine for "jump to # roughly here"); exact-quote positioning is a future tier. From 57550a6227188baadba4fe25d918d8c44dfd2f78 Mon Sep 17 00:00:00 2001 From: jp Date: Sun, 14 Jun 2026 07:09:59 -0700 Subject: [PATCH 2/2] feat(miner): thread symbol_header_prefix through process_file + mine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Gemini review on #1508. The kwarg was reachable only by calling chunk_text directly; wire it through the standard mining entry points so the enrichment seam is usable from mine() without forking the path: mine -> _mine_impl -> process_file -> chunk_text symbol_header_prefix is an optional kwarg (default None) at every level, fully backward-compatible — no existing call site changes behavior. Also from the review: - Document the re-mine caveat in chunk_text + process_file docstrings: file_already_mined keys only on source_mtime + NORMALIZE_VERSION, so changing the callback does not auto-invalidate already-mined files; re-mine needs --force or a NORMALIZE_VERSION bump. - Rename the documented callback arg chunk_text -> chunk in the docstring to avoid confusion with the function name: (chunk, source_file, chunk_index) -> str. Tests: prove symbol_header_prefix flows mine -> process_file -> chunk_text, that the default (no kwarg) reaches process_file as None, that a non-empty header is prepended with a blank-line separator, and that the default-None path is byte-identical to the no-kwarg call. Co-Authored-By: Claude Opus 4.8 --- mempalace/miner.py | 29 ++++++++++- tests/test_miner.py | 114 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) diff --git a/mempalace/miner.py b/mempalace/miner.py index 4b79091c45..5b9adee29a 100644 --- a/mempalace/miner.py +++ b/mempalace/miner.py @@ -494,7 +494,7 @@ def chunk_text( ``MIN_CHUNK_SIZE`` are used. Validated as positive ints; an overlap >= size is rejected to avoid an infinite loop. symbol_header_prefix: optional callable - ``(chunk_text, source_file, chunk_index) -> str``. When + ``(chunk, source_file, chunk_index) -> str``. When supplied, the returned header is prepended to each chunk with a blank line separator before storage. Lets AST-lite symbol enrichment (function names, class paths, imports) @@ -503,6 +503,12 @@ def chunk_text( original behavior exactly. Discussed in MemPalace/mempalace#1384. + Caveat: ``file_already_mined`` keys only on ``source_mtime`` + and ``NORMALIZE_VERSION``, so changing the callback (or the + header it returns) does NOT invalidate files already mined + with a different prefix. Re-mine those files with ``--force`` + or bump ``NORMALIZE_VERSION`` to pick up the new headers. + Returns: list of ``{"content": str, "chunk_index": int, "line_start": int, "line_end": int}``. ``line_start`` / ``line_end`` are 1-indexed @@ -1309,6 +1315,7 @@ def process_file( chunk_overlap: int = None, min_chunk_size: int = None, max_chunks_per_file: Optional[int] = None, + symbol_header_prefix=None, ) -> tuple: """Read, chunk, route, and file one file. @@ -1318,6 +1325,14 @@ def process_file( 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). + + ``symbol_header_prefix`` is an optional callable forwarded to + :func:`chunk_text` for per-chunk header enrichment; ``None`` (default) + preserves current behavior exactly. Caveat: ``file_already_mined`` + keys only on ``source_mtime`` and ``NORMALIZE_VERSION``, so changing + the callback (or the header it returns) does NOT trigger a re-mine of + files already filed under a different prefix — use ``--force`` or bump + ``NORMALIZE_VERSION`` to pick up the new headers. """ effective_min = min_chunk_size if min_chunk_size is not None else MIN_CHUNK_SIZE @@ -1342,6 +1357,7 @@ def process_file( chunk_size=chunk_size, chunk_overlap=chunk_overlap, min_chunk_size=min_chunk_size, + symbol_header_prefix=symbol_header_prefix, ) effective_cap = _resolve_max_chunks_per_file(max_chunks_per_file) @@ -1568,6 +1584,7 @@ def mine( include_ignored: list = None, files: list = None, max_chunks_per_file: Optional[int] = None, + symbol_header_prefix=None, ): """Mine a project directory into the palace. @@ -1581,6 +1598,12 @@ def mine( :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). + + ``symbol_header_prefix`` is an optional per-chunk header callable + forwarded to :func:`chunk_text` via :func:`process_file`; ``None`` + (default) preserves current behavior exactly. See :func:`chunk_text` + for the signature and the re-mine caveat (changing it does not + invalidate already-mined files). """ if dry_run: return _mine_impl( @@ -1594,6 +1617,7 @@ def mine( include_ignored=include_ignored, files=files, max_chunks_per_file=max_chunks_per_file, + symbol_header_prefix=symbol_header_prefix, ) # MineAlreadyRunning propagates so the CLI can render a clear holder-aware @@ -1611,6 +1635,7 @@ def mine( include_ignored=include_ignored, files=files, max_chunks_per_file=max_chunks_per_file, + symbol_header_prefix=symbol_header_prefix, ) @@ -1625,6 +1650,7 @@ def _mine_impl( include_ignored: list = None, files: list = None, max_chunks_per_file: Optional[int] = None, + symbol_header_prefix=None, ): from .config import MempalaceConfig @@ -1700,6 +1726,7 @@ def _mine_impl( # otherwise a malformed env var would emit its warning # per file. max_chunks_per_file=effective_chunk_cap, + symbol_header_prefix=symbol_header_prefix, ) except KeyboardInterrupt: # Re-raise so the outer handler prints the summary; we diff --git a/tests/test_miner.py b/tests/test_miner.py index 34ceff6dc9..ded225bd6a 100644 --- a/tests/test_miner.py +++ b/tests/test_miner.py @@ -1654,6 +1654,120 @@ def fake_process_file(*args, **kwargs): assert captured["max_chunks_per_file"] == 0 +def test_mine_plumbs_symbol_header_prefix_to_process_file(tmp_path): + """``mine(symbol_header_prefix=cb)`` reaches ``process_file`` as + kwarg=cb. Guards the wiring ``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" + + def my_header(chunk, source_file, chunk_index): + return "" + + captured = {} + + def fake_process_file(*args, **kwargs): + captured["symbol_header_prefix"] = kwargs.get("symbol_header_prefix") + return (1, "general", None) + + with patch("mempalace.miner.process_file", side_effect=fake_process_file): + mine(str(project_root), str(palace_path), symbol_header_prefix=my_header) + + assert captured["symbol_header_prefix"] is my_header + + +def test_mine_default_symbol_header_prefix_is_none_at_process_file(tmp_path): + """The default ``mine(...)`` call (no ``symbol_header_prefix``) reaches + ``process_file`` with ``symbol_header_prefix=None`` — current behavior is + preserved when the caller does not opt in.""" + 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["symbol_header_prefix"] = kwargs.get("symbol_header_prefix", "MISSING") + return (1, "general", None) + + with patch("mempalace.miner.process_file", side_effect=fake_process_file): + mine(str(project_root), str(palace_path)) + + assert captured["symbol_header_prefix"] is None + + +def test_process_file_forwards_symbol_header_prefix_to_chunk_text(tmp_path): + """``process_file`` forwards its ``symbol_header_prefix`` kwarg to + ``chunk_text`` unchanged, completing the ``process_file -> chunk_text`` + link of the wiring.""" + from unittest.mock import patch + + from mempalace.miner import process_file + + src = tmp_path / "f.py" + src.write_text("def fn():\n print('hi')\n" * 40) + + def my_header(chunk, source_file, chunk_index): + return "" + + captured = {} + + def fake_chunk_text(*args, **kwargs): + captured["symbol_header_prefix"] = kwargs.get("symbol_header_prefix") + return [] + + with patch("mempalace.miner.chunk_text", side_effect=fake_chunk_text): + process_file( + filepath=src, + project_path=tmp_path, + collection=None, + wing="w", + rooms=[{"name": "general", "description": "g"}], + agent="test", + dry_run=True, + symbol_header_prefix=my_header, + ) + + assert captured["symbol_header_prefix"] is my_header + + +def test_chunk_text_symbol_header_prefix_prepends_header(): + """When supplied, the callback's non-empty return is prepended to each + chunk with a blank-line separator; an empty return is a per-chunk no-op.""" + from mempalace.miner import chunk_text + + content = "\n".join(f"line {i}" for i in range(1, 401)) + + def header(chunk, source_file, chunk_index): + return f"HDR:{chunk_index}" + + enriched = chunk_text( + content, "/x.py", chunk_size=800, chunk_overlap=80, symbol_header_prefix=header + ) + assert enriched, "should produce chunks" + for c in enriched: + assert c["content"].startswith(f"HDR:{c['chunk_index']}\n\n") + + +def test_chunk_text_default_none_byte_identical_to_no_kwarg(): + """``symbol_header_prefix=None`` (the default) yields byte-identical + chunk content to the no-kwarg call — fully backward-compatible.""" + from mempalace.miner import chunk_text + + content = "\n".join(f"line {i}" for i in range(1, 401)) + baseline = chunk_text(content, "/x.py", chunk_size=800, chunk_overlap=80) + with_default = chunk_text( + content, "/x.py", chunk_size=800, chunk_overlap=80, symbol_header_prefix=None + ) + assert [c["content"] for c in with_default] == [c["content"] for c in baseline] + + def test_mine_arbitrary_exception_prints_summary_and_reraises(tmp_path, capsys): """A non-KeyboardInterrupt exception mid-mine must surface a summary banner before propagating, so users don't see a silent exit-0 with no