Skip to content
Open
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
65 changes: 57 additions & 8 deletions mempalace/miner.py
Original file line number Diff line number Diff line change
Expand Up @@ -477,19 +477,43 @@ def chunk_text(
chunk_size: int = None,
chunk_overlap: int = None,
min_chunk_size: int = None,
*,
symbol_header_prefix=None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The symbol_header_prefix parameter is added here but not yet exposed in the process_file function (line 882) or the mine entry point. This makes the enrichment feature inaccessible for the standard mining workflow without further modifications to those functions. Additionally, since file_already_mined only checks source_mtime and NORMALIZE_VERSION, changes to the logic within a provided callback will not automatically trigger a re-mine of already processed files.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch on both. Wired symbol_header_prefix through process_file and the mine entry point (mine -> _mine_impl -> process_file -> chunk_text) as an optional kwarg, default None = exactly current behavior, in 57550a6 — so it's now reachable from the standard mining workflow without forking the path. On the cache key: agreed it's out of scope to solve here, so I documented it instead — both chunk_text and process_file docstrings now note that file_already_mined keys only on source_mtime + NORMALIZE_VERSION, so changing the callback does not auto-invalidate already-mined files; a re-mine needs --force or a NORMALIZE_VERSION bump. Added tests proving the kwarg flows end-to-end and that the default path is byte-identical to the no-kwarg call.

) -> 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, 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.

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
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
Expand Down Expand Up @@ -543,6 +567,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.
Expand Down Expand Up @@ -1287,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.

Expand All @@ -1296,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

Expand All @@ -1320,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)
Expand Down Expand Up @@ -1546,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.

Expand All @@ -1559,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(
Expand All @@ -1572,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
Expand All @@ -1589,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,
)


Expand All @@ -1603,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

Expand Down Expand Up @@ -1678,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
Expand Down
114 changes: 114 additions & 0 deletions tests/test_miner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading