PR #1555 (format coverage + virtual line numbering) merged with twelve
inline polish comments from Copilot + gemini-code-assist that weren't
load-bearing enough to block the original ship but are real cleanups.
This PR addresses them.
Twelve items in scope; one item (drawer ID delimiter — Copilot #13) is
deferred to its own dedicated PR because it's a breaking schema change
that requires migration design beyond the scope of a polish PR.
## Behavioral fixes (5 items, RED-tested first)
1. **FileNotFoundError vs broken symlink (Copilot #8).** ``extract_text``
previously mapped every ``FileNotFoundError`` from ``stat()`` to
``SKIP_BROKEN_SYMLINK``. That's misleading for the common case of a
regular file deleted between scan and extract. Now distinguishes:
``SKIP_BROKEN_SYMLINK`` only when ``p.is_symlink()`` is true;
``SKIP_UNREADABLE`` otherwise.
2. **``file_already_mined`` extract_mode scoping (Copilot #11, #12).**
Both call sites in ``mine_formats`` and ``_file_chunks_locked`` now
pass ``extract_mode="format"``. Previously the format miner could
falsely treat drawers from project / convo miner on the same source
file as "already mined" (and vice versa). Scopes idempotency to the
correct drawer subset.
3. **Sentinel skip for transient missing-dep statuses (Copilot #14).**
New ``_TRANSIENT_MISSING_DEP_STATUSES`` set + ``_register_skip_sentinel_if_appropriate``
helper. Skip variants like ``SKIP_NO_MARKITDOWN`` /
``SKIP_NO_STRIPRTF`` / ``SKIP_MISSING_FORMAT_DEPS`` /
``SKIP_NETWORK_TIMEOUT`` no longer write the "already-mined" sentinel.
Otherwise installing the missing extra later wouldn't trigger a re-mine.
4. **Outer ``except Exception`` in ``mine_formats`` (Gemini #5).** The
outer try around the loop previously caught only ``KeyboardInterrupt``,
leaving any setup-time error (e.g., ``scan_formats`` raising) to
propagate as a bare traceback. Now catches ``Exception`` defensively,
logs it, prints a partial-progress summary, and lets the ``finally``
PID-cleanup run. Mirrors miner.py's belt-and-suspenders pattern.
5. **Thread user's ``chunk_size`` / ``chunk_overlap`` / ``min_chunk_size``
through to ``chunk_text`` (Gemini #3).** ``MempalaceConfig`` was loaded
only to validate readability; users who tuned their config saw no
effect in format-mode mining. Now properly threaded.
## Trivial cleanups (5 items)
6. **Path expanduser in ``extract_text`` (Copilot #7).** ``Path(path)`` →
``Path(path).expanduser()`` so CLI inputs like ``~/docs/file.pdf``
resolve correctly.
7. **Path expanduser+resolve in ``scan_formats`` (Copilot #9).** Same
fix; ``~/docs`` and relative paths now work consistently.
8. **Use resolved ``format_path`` in ``mine_formats`` (Copilot #10).**
``scan_formats(format_dir)`` → ``scan_formats(format_path)`` so the
already-resolved path is used.
9. **``render_with_line_numbers`` type annotation (Copilot #15).**
``text: "str | None"`` reflects the documented + tested ``None``
handling.
10. **Test + docs claims (Copilot #16, #17, #18).** Stale framings
removed:
- ``docs/format-coverage.md`` — 14 fringe cases + "see the file for
the current test inventory" (no more frozen test count).
- ``tests/test_line_numbers.py`` — drops "proposed for mempalace
3.3.6" + "run from the proposal directory" references.
- ``tests/test_format_miner.py`` — drops "MarkItDown is mocked
throughout" (live integration tests exist) + proposal-directory
framing.
## Module-level hoists (enables clean test patching)
- ``MempalaceConfig`` (from ``.config``) hoisted from lazy local import
to module-level so tests can patch ``mempalace.format_miner.MempalaceConfig``.
- ``chunk_text`` (from ``.miner``) hoisted similarly.
Both follow the pattern PR #1565 used for ``compute_hallways_for_wing``.
## Complexity refactor
Extracted ``_print_mine_summary`` from ``mine_formats`` so the orchestrator
stays under the project's ``max-complexity = 25`` ceiling (per
``pyproject.toml [tool.ruff.lint.mccabe]``). Behavior unchanged; pure
extraction.
## Out of scope (intentionally deferred)
- **Drawer ID delimiter collision (Copilot #13)** — ``f"{source_file}{chunk_index}"``
can theoretically collide (``"/path/a1" + "23"`` == ``"/path/a" + "123"``).
Fixing this is a breaking schema change to drawer IDs and requires a
migration plan; will land as its own PR after design.
- The four bot comments that were ALREADY addressed by amendment #3
before the PR #1555 merge (``_SKIP_DIRS`` dedup, ``scan_formats``
symlink skip, ``source_mtime`` tracking, hall+entities metadata) —
no action needed; verified during audit.
## Tests (RED-first)
Six new RED-first tests in ``tests/test_format_miner.py``:
test_extract_text_nonexistent_regular_file_returns_unreadable_not_broken_symlink
test_mine_formats_passes_extract_mode_format_to_file_already_mined
test_mine_formats_does_not_write_sentinel_for_skip_no_markitdown
test_mine_formats_does_not_write_sentinel_for_skip_missing_format_deps
test_mine_formats_catches_unexpected_exception_and_prints_summary
test_mine_formats_threads_chunk_size_from_user_config
All six RED before this commit (failures correctly identified the bugs
they're targeting), all six GREEN after.
One existing test (``test_mine_formats_continues_after_per_file_error``)
updated to patch the new module-level binding
``mempalace.format_miner.chunk_text`` instead of the old
``mempalace.miner.chunk_text`` source location, and to accept the
``**kwargs`` the call now passes through. Behavior unchanged.
## Verification
pytest -q (full mempalace suite)
→ 2065 passed, 3 skipped, 0 regressions
ruff check mempalace/format_miner.py mempalace/searcher.py tests/
→ All checks passed!
ruff format --check ...
→ 4 files already formatted (pinned 0.15.9)
mine_formats complexity
→ ≤ 25 (under the project ceiling)
Two additive features, both following the same read-time-transform pattern:
1. Virtual line numbering — new render_with_line_numbers() and
extract_line_range() in mempalace/searcher.py. Closet pointers like
2026-01-18:L55-L72 resolve to drawer slices rendered with [55] through
[72] line prefixes without modifying any stored content. Lines already
prefixed with [<digits>] pass through unchanged. Pure functions, no I/O.
2. Format coverage (mempalace mine --mode extract) — new mempalace/format_miner.py
reads binary office formats and files drawers via the lock + purge +
upsert pattern convo_miner uses. Source files never modified.
Per-format transformer routing: MarkItDown 0.1.5 does not actually
convert .rtf (returns raw control codes unchanged, verified live), so
.rtf is routed to striprtf which does convert. Other formats stay on
MarkItDown:
.pdf .docx .pptx .xlsx .epub -> MarkItDown
.rtf -> striprtf
13 fringe cases handled with dedicated ExtractionStatus codes:
SKIP_NO_MARKITDOWN, SKIP_NO_STRIPRTF, SKIP_TOO_LARGE, SKIP_CLOUD_ONLY,
SKIP_ENCRYPTED, SKIP_EMPTY, SKIP_PERMISSION, SKIP_BROKEN_SYMLINK,
SKIP_UNRECOGNIZED, SKIP_EXTRACTION_ERROR, SKIP_NETWORK_TIMEOUT,
SKIP_UNREADABLE, plus the encoding fallback handled internally.
Drawers carry ingest_mode=extract + extract_mode=format so they are
distinguishable from project / convo drawers in the palace.
Both transformers are optional extras: pip install mempalace[extract].
MarkItDown requires Python >= 3.10 (env marker ensures it only installs
where supported; Python 3.9 users still get RTF coverage via striprtf).
Tests: 94 new (21 line-numbering + 73 format-miner, including 9
mine_formats orchestrator tests). Full mempalace suite still green
(1992 passed locally). Coverage 86% on mempalace/format_miner.py.
ruff check + ruff format clean on the pinned 0.15.9.
Live verification: mempalace mine --mode extract on a directory with 2
RTFs + 1 PDF produced 90 drawers correctly; mempalace search found
content from both file types in the resulting wing.
Documented limits (out of scope for 3.3.6): custom PDF parsers, OCR on
scanned PDFs, DRM-locked files, pathological corrupt files. These get
reported via skip codes and skipped.
Refs: docs/format-coverage.md, docs/virtual-line-numbering.md
Milla J