`mempalace_diary_write` returns an `entry_id` for every diary entry, but
for entries large enough to be chunked that id was unusable: get_drawer,
update_drawer and delete_drawer all answered "Drawer not found", and
list_drawers showed the entry as N unrelated chunk rows.
Two metadata conventions never met. The diary chunking path stamped
`parent_entry_id` on each chunk, while the logical-id read paths added in
#1782 query only `parent_drawer_id`. Both keys mean the same thing --
"physical chunk of this logical drawer" -- so chunk groups written by
diary_write were invisible to logical-id resolution. Same bug class as
#1763, which #1782 fixed for `add_drawer` drawers only.
Read paths now resolve either key via `_PARENT_ID_KEYS`:
- `_logical_chunk_group()` matches both with an `$or` (fixes get /
update / delete). All four backends support `$or`.
- `_collapse_drawer_rows()` groups on either (fixes list_drawers, which
the `$or` alone does not cover).
- `searcher._result_drawer_id()` resolves either, so a hit on a chunked
diary entry reports the id that fetches the whole entry rather than
the single chunk that matched.
New diary writes also stamp `parent_drawer_id` alongside
`parent_entry_id` so the two conventions converge going forward. Because
the read paths still accept the `parent_entry_id`-only shape, palaces
written before this fix are repaired with no data migration.
Diary chunks are written without `source_file`, so neighbor expansion
(#1580) returns early on them and is unaffected by the added key.
Also drops the comment telling callers to iterate `chunk_ids` (it
documented the bug as intended behavior) and a stale claim that search
rejoins chunks via `parent_entry_id` -- no search code read that key.
Address actionable #2129 bot feedback and the Windows closet KeyError:
- Reopen immutable sqlite_exact readers only when both WAL sidecars exist
(partial pair keeps the clean snapshot instead of failing the reconnect).
- Retry get_collection without options when plugin backends reject the kwarg.
- Stamp multi-conversation content_hash only on chunk 0 to avoid O(N²) meta.
- Always include results: [] on search error envelopes so callers never KeyError.
- Clearer hybrid-search assertions in the closet isolation test.
Igor's review on PR #1584 (2026-05-22) flagged four issues:
1. The feature wasn't wired into any production caller — the new
``drawer_metas`` kwarg on ``build_closet_lines`` had no real
consumer in ``miner.py`` / ``diary_ingest.py``, so the 4-segment
pointer form only existed in tests. Real palaces kept emitting
the legacy 3-segment shape.
2. ``_extract_content_date`` hallucinated dates on benign inputs.
``dateutil.parser.parse(fuzzy=True)`` would accept anything with
digits and return a plausible-looking but wrong date —
``Version 3.3.6`` → ``2006-03-03``, ``Tested with 1000 drawers``
→ ``1000-05-22``, ``tmp_random_file_5`` → ``2026-05-05``, etc.
Mtime almost never got reached because fuzzy returned *something*
from filename or body first. Bad dates were silently persisted
to ChromaDB.
3. ``python-dateutil`` was an undeclared dependency, available only
transitively via ``chromadb → kubernetes → python-dateutil``. Not
a contract — upstream kubernetes has been trending toward
stdlib-only.
4. Two-digit-year disambiguation (70 → 19xx / 00-69 → 20xx) had no
test pinning the boundary.
This commit addresses all four.
## Changes
### Issue 2 — kill the hallucination (the load-bearing fix)
``mempalace/miner.py``:
- New ``_VALID_DATE_RE`` gate. Three accepted shapes (all require a
4-digit year explicitly):
1. Numeric YYYY-MM-DD with ``[-/.\\s]`` separators
(covers ISO and space-normalized filenames)
2. Month-name + day + year ("November 8 2024", "Nov 8 2024")
3. Day + month-name + year ("8 November 2024")
Partial dates ("2024-06", "April 6", "notes.2024") are
DELIBERATELY rejected — without all three components we'd pad from
today's date, which is hallucination not extraction.
- ``_try_filename_date`` and ``_try_content_body_date`` now run the
gate BEFORE invoking dateutil, and pass ``fuzzy=True`` is REMOVED.
Dateutil only runs in strict mode on a substring the gate already
validated.
### Issue 1 — wire the feature into production
``mempalace/miner.py`` batched-upsert path:
- Accumulate ``batch_metas`` across all batches into ``all_metas``
- Pass ``drawer_metas=all_metas`` to ``build_closet_lines``
End-to-end integration test added that mines a real file with a
filename-derived content date and asserts the produced closet
documents contain the 4-segment pointer with that date.
``diary_ingest.py`` is left as-is for this PR. Diary entries are
entry-keyed, not chunk-keyed — they carry no natural
``line_start`` / ``line_end``, so the 4-segment form would return
None for them regardless. Wiring the diary path can land cleanly in
a follow-up once Tier 6a gains an "approximate line range for diary
entries" story.
### Issue 3 — declare the dateutil dependency
``pyproject.toml``: add ``python-dateutil>=2.8`` to
``[project].dependencies``. One-line change; cheaper than the
stdlib-only refactor alternative and keeps the natural-language
recall surface.
### Issue 4 — pin the two-digit-year boundary
Four new tests cover the 1969/1970/1999/2000 corner cases of the
slash-date locale heuristic.
## Tests added (RED-first then GREEN)
tests/test_miner.py::TestExtractContentDate (11 new):
Hallucination cases verbatim from Igor's review:
- test_no_hallucination_junk_filename_with_trailing_digit
- test_no_hallucination_untitled_with_index
- test_no_hallucination_filename_year_only
- test_no_hallucination_filename_year_and_month_only
- test_no_hallucination_content_with_issue_number
- test_no_hallucination_content_with_count
- test_no_hallucination_content_with_version_number
Two-digit-year boundary cases:
- test_two_digit_year_69_is_2069
- test_two_digit_year_70_is_1970
- test_two_digit_year_99_is_1999
- test_two_digit_year_00_is_2000
tests/test_closets.py::TestMinerClosetRebuild (1 new):
- test_production_miner_emits_4_segment_pointers_with_content_date
(regression for Issue #1 — real ``mine()`` end-to-end produces
4-segment closet pointers via the new ``drawer_metas`` wiring)
## Verification
pytest tests/test_miner.py tests/test_closets.py
tests/test_format_miner.py tests/test_palace.py
→ 242 passed, 2 skipped, 0 regressions
pytest tests/test_miner.py::TestExtractContentDate
→ 26 passed (15 prior + 11 new)
pytest tests/test_closets.py::TestMinerClosetRebuild
→ end-to-end wiring test GREEN
Sanity (Igor's exact repros):
"tmp_random_file_5" → None (was: 2026-05-05)
"untitled-1" → None (was: 2026-05-01)
"notes.2024.md" → None (was: 2024-05-22)
"2024-06.md" → None (was: 2024-06-22)
"Bug fix for issue 42 in module 7" → None (was: 2042-07-22)
"Tested with 1000 drawers" → None (was: 1000-05-22)
"Version 3.3.6 released" → None (was: 2006-03-03)
Real dates still extract correctly:
"2024-11-08.md" → "2024-11-08"
"April-6th-2011-notes.md" → "2011-04-06"
"Nov-8-2024.md" → "2024-11-08"
ruff check + ruff format --check (pinned 0.15.9)
→ All checks passed
OrbStack triple-Linux verify (Py 3.9 / 3.11 / 3.13)
→ all targeted tests pass; python-dateutil installs explicitly
via the new declared dependency.
Closet pointer lines gain a 4th pipe-separated segment of shape
``YYYY-MM-DD:Lstart-Lend`` so retrieval can jump straight to the right
span in the source file instead of opening the whole drawer. The date
is selected via a content-aware hierarchy so legacy ingests (where
``filed_at`` is misleading) still produce honest pointers when the
content itself carries a date.
The legacy 3-segment format remains the fallback for drawers without
the new metadata, so existing palaces keep working without a re-mine.
## Pointer shape
Before:
topic|entities|→drawer_ids
After (when drawer metadata carries line range + a date):
topic|entities|2024-11-08:L42-L78|→drawer_ids
Backward compat: when ``drawer_metas`` is not passed, or the first
meta lacks ``line_start`` / ``line_end``, ``build_closet_lines`` emits
the legacy 3-segment form unchanged.
## Date-source hierarchy (NEW)
A new helper ``mempalace.miner._extract_content_date(source_file, content)``
selects a date once per file and stores it as ``content_date`` on every
drawer mined from that file. Priority order, first match wins:
1. **Filename** — ISO regex on stem, then dateutil fuzzy parse for
natural-language formats. Handles ``2024-11-08.md``,
``April-6th-2011-notes.md``, ``Nov-8-2024.md``, etc.
2. **YAML frontmatter** — looks at ``date`` / ``created`` /
``published`` fields. yaml.safe_load already parses ISO dates as
``datetime.date`` objects; dateutil handles string values.
3. **Content body, first ~10 lines** —
a. ISO regex (``YYYY-MM-DD`` / ``YYYY/MM/DD`` / ``YYYY.MM.DD``)
b. Slash dates with locale auto-disambiguation: if any
first-number > 12 appears anywhere in the head, lock the
file's locale to DD/MM (the only consistent reading);
otherwise default to US MM/DD for two-digit-year compactness.
c. dateutil fuzzy parse for natural-language
(``November 8, 2024``, ``8th November 2024``, etc.)
4. **Filesystem mtime** — ``os.path.getmtime`` formatted as
``YYYY-MM-DD``.
5. **None** — caller (``_build_drawer_metadata``) leaves
``content_date`` absent; ``build_closet_lines`` falls back to the
``filed_at`` ingestion timestamp's date portion.
``_build_date_line_segment`` in ``palace.py`` now prefers ``content_date``
when present, otherwise uses ``filed_at[:10]``. This is the load-bearing
change that makes Tier 6a meaningful for legacy content — a Nov 8 2024
transcript mined today now emits ``2024-11-08:L42-L78`` instead of
``2026-05-22:L42-L78``.
## Changes
1. ``mempalace/miner.py::chunk_text`` — each chunk dict now also carries
``line_start`` / ``line_end`` (1-indexed line numbers in the stripped
source). Computed via ``content[:offset].count("\n") + 1`` —
approximate locator accurate to ±1 line at chunk boundaries.
2. ``mempalace/miner.py::_extract_content_date`` (NEW) + helpers
(``_try_filename_date``, ``_try_frontmatter_date``,
``_try_content_body_date``, ``_try_mtime_date``). Pure functions, no
I/O beyond what's named. Uses lazy imports of ``dateutil`` and
``yaml`` to keep top-level import surface unchanged.
3. ``mempalace/miner.py::_build_drawer_metadata`` — gains
``line_start`` / ``line_end`` / ``content_date`` optional kwargs.
When provided, stored in drawer metadata; when omitted, keys are
absent from the dict — backward compatible.
4. ``mempalace/miner.py`` batched-upsert path — calls
``_extract_content_date(source_file, content)`` ONCE per file (not
once per chunk) and passes the result through to every chunk's
``_build_drawer_metadata`` call.
5. ``mempalace/format_miner.py::_file_chunks_locked`` — gains a
``content`` param so the format-miner path can also run
``_extract_content_date`` on the full extracted text. Plumbs
``content_date`` into every batched drawer's metadata. Caller passes
``text`` (the full extracted markdown).
6. ``mempalace/palace.py::build_closet_lines`` — accepts optional
``drawer_metas`` (parallel to ``drawer_ids``). When present and the
first meta has both ``line_start`` and ``line_end``, the new
segment is spliced in; otherwise legacy 3-segment form. Helper
``_build_date_line_segment`` prefers ``content_date`` over
``filed_at`` for the date portion.
## Tests added (RED-first, then GREEN)
tests/test_miner.py::TestChunkTextLineRanges (4 tests)
tests/test_miner.py::TestBuildDrawerMetadataLineRange (2 tests)
tests/test_miner.py::TestExtractContentDate (15 tests)
- filename ISO / natural-language / compact
- YAML frontmatter date / created
- Claude session preamble
- ISO + natural-language content body
- Slash-date locale disambiguation (DD/MM lock vs default MM/DD)
- Priority order (filename > frontmatter > content > mtime)
- mtime fallback
- Graceful None returns for missing file / empty content
tests/test_closets.py::TestBuildClosetLines (5 new tests)
- includes_date_line_segment_when_metas_provided
- falls_back_to_3_segment_format_when_metas_missing (compat)
- falls_back_to_3_segment_format_when_metas_lack_line_keys (compat)
- date_segment_uses_filed_at_date_portion_only
- content_date_preferred_over_filed_at (new — Tier 6a hierarchy)
26 new tests total. All RED before this commit, all GREEN after.
## Verification
pytest tests/test_miner.py::TestChunkTextLineRanges
tests/test_miner.py::TestBuildDrawerMetadataLineRange
tests/test_miner.py::TestExtractContentDate
tests/test_closets.py::TestBuildClosetLines
→ 32 passed
pytest tests/test_miner.py tests/test_closets.py
tests/test_format_miner.py tests/test_palace.py
→ 230 passed, 2 skipped, 0 regressions in directly-affected files
ruff check + ruff format --check
→ All checks passed, 5 files already formatted
End-to-end smoke test on a YAML-frontmatter file (date: 2024-11-08,
mined "today"):
content_date extracted: 2024-11-08
chunks produced: 1
drawer meta: content_date=2024-11-08, filed_at='2026-05-22...', lines=1-15
closet pointer:
"conversation with lumi about brands of dog food|Filler;Lumi;Aya|2024-11-08:L1-L15|→drawer_a,drawer_b"
→ content_date correctly preferred over filed_at end-to-end.
OrbStack triple-Linux verify (Py 3.9 / 3.11 / 3.13)
→ 32 targeted Tier 6a + content-date tests pass on each.
## Out of scope (deliberate)
- No schema migration of existing drawers. Pre-Tier-6a drawers lack
``line_start`` / ``content_date`` and silently fall back to the
legacy 3-segment pointer shape.
- No re-mine required. Users keep existing palaces; new files mined
after this lands carry the new fields.
- No drawer ID change (Task #80, separate concern).
- No exact-quote line positioning within a chunk (future tier).
- No NLP-based date extraction (only regex + dateutil fuzzy).
- Linguistic-similarity / concept-recognition retrieval (L5 zettal
layer) is the natural next step but lives in its own PR family.
Resolves conflicts from 60-commit divergence:
- tests/test_closets.py: assertion reformat — kept develop's ruff-format-
preferred multi-line shape (functionally identical).
- tests/test_palace_graph_tunnels.py: both branches added a new test
class at end-of-file (this PR's TestTunnelFileFollowsConfig + develop's
TestEntityTunnels from #1564). Kept both, no overlap.
Local: ruff check / format pass; full pytest suite passes
(2113 passed, 3 skipped).
Co-Authored-By: Igor Lins e Silva <4753812+igorls@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four sites passed content to `collection.upsert(documents=[...])` or
`collection.add(documents=[...])` without per-drawer size cap, hitting
the same `RuntimeError: Invalid buffer size` crash class from the
embedding model's attention buffer:
- `general_extractor.extract_memories`: post-classification slicer
that propagates `memory_type` to every slice. New `chunk_size`
parameter defaulting to `DEFAULT_CHUNK_SIZE` and resolved from
`MempalaceConfig.chunk_size` by the caller.
- `diary_ingest.ingest_diaries`: per-entry drawers via existing
`_split_entries`, with character-chunk fallback for any single
entry larger than `chunk_size`. New `_diary_drawer_id_entry`
helper carries (entry_idx, entry_chunk_idx). `chunk_index` in
metadata is a global counter across the file so
`searcher._expand_with_neighbors` stitches sibling chunks
regardless of entry boundary. Upsert is batched atomic per file
(one call carrying every entry/chunk) so a mid-pass embedding
failure cannot half-write the day. Auto-purge on full rebuild
deletes prior-pass drawers via `where={"source_file": ...}`,
which also migrates pre-#1539 legacy `drawer_diary_` IDs as a
side effect of normal use.
- `mcp_server.tool_diary_write`: split oversized entries into
bounded per-chunk drawers via a single batched `col.add` (atomic,
no half-write on embedding failure). `col.add` is intentional:
`entry_id` is timestamp-based with microsecond precision, so a
duplicate is a same-microsecond clash that should surface rather
than silently overwrite.
- `mcp_server.tool_add_drawer`: same crash class on the more common
add-drawer surface (100 KB sanitize cap, 125x `CHUNK_SIZE`).
Chunked path mirrors Site 3 with batched atomic `col.upsert`,
per-chunk `parent_drawer_id` + `chunk_index` metadata, and a
dual-id idempotency probe (last chunk for atomicity, legacy
`drawer_id` for pre-#1539 single-row backwards-compat). Return
shape additive: `chunks` always present, `chunk_ids` on the
chunked path. `tool_get_drawer` / `tool_delete_drawer` against
the logical handle report "not found" on the chunked path;
callers iterate `chunk_ids` or query `parent_drawer_id`.
Chunk id width is `:06d` so even a single-digit `chunk_size`
config cannot lex-sort chunks out of order.
Three changes addressing #1469 CI red + Gemini perf review:
1. ruff format (0.4.x) on tests/test_closets.py and
tests/test_palace_graph_tunnels.py — the lint job pins
ruff>=0.4.0,<0.5 and was flagging format drift.
2. Replace %r with '%s' in legacy / corrupt tunnel-file warnings.
On Windows %r escapes backslashes in repr, so
test_load_tunnels_warns_on_orphaned_legacy_file's
'str(legacy) in caplog.text' assertion was failing on
test-windows even though the warning was firing.
3. Address gemini-code-assist review on #1469: pass a single
MempalaceConfig() through _get_tunnel_file / _load_tunnels /
_save_tunnels per create_tunnel call instead of each helper
re-instantiating its own (which re-reads mempalace.yaml from
disk). Helpers keep their config=None defaults so external
callers and existing tests are unaffected.
One-time mechanical reformat so `ruff format --check .` passes under the
newly pinned ruff. Layout only (assert-message parenthesization etc.),
no behavior change. 29 files: 28 under tests/ + 1 tools helper, no core
mempalace/ modules. Produced by `ruff format .`.
palace_graph._TUNNEL_FILE was a module-level constant initialised from
os.path.expanduser("~") + "/.mempalace/tunnels.json", ignoring the
MempalaceConfig.palace_path config (and MEMPALACE_PALACE_PATH env var)
that drawers, KG, and every other piece of palace state honour. Under
any setup where $HOME and the configured palace diverge — subagent
profiles, sandboxes, multi-tenant hosts, container mounts moving the
palace to /srv/ — drawers landed in the configured palace while
tunnels silently landed in a different file invisible to other
processes touching the same palace.
Replace the constant with _get_tunnel_file(config=None) deriving the
path from a new MempalaceConfig.tunnel_file property (sibling of
palace_path). Default install unchanged because default palace_path
is ~/.mempalace/palace whose sibling tunnels.json is the legacy path.
Add a _legacy_tunnel_file() helper and a one-line WARNING in
_load_tunnels for the case where the configured tunnel file is missing
but the pre-fix hardcoded path has one. No auto-migration — silently
merging tunnel state across two locations risks clobbering newer data.
fix(graph): validate explicit-tunnel endpoints exist (#1468)
create_tunnel previously only validated that wing/room names were
non-empty strings; nothing queried chroma to confirm at least one
drawer carried matching {wing, room} metadata. Pointing an explicit
tunnel at a phantom room silently succeeded. Combined with #1467's
read-bubble, an agent could create_tunnel → list_tunnels and have both
calls return its own bogus write, self-confirming a tunnel that didn't
exist in the shared palace.
create_tunnel now calls _check_room_exists(wing, room, col) for both
endpoints before persisting an explicit tunnel; zero rows raises
ValueError naming the endpoint. Three deliberate carve-outs:
- kind != "explicit" skips validation because topic tunnels use
synthetic topic:<name> room ids that don't correspond to real rooms
- _get_collection returning None (palace not yet created, transient
failure, tests without backend) skips validation rather than
fail-closed — matches tolerance pattern used throughout palace_graph
- Query exceptions are logged and treated as 'can't verify, allow' so
a flaky index doesn't block legitimate writes
Behaviour change: callers that previously created tunnels pointing at
empty rooms (scaffolding before mining) will now raise. File the
drawer first, then create the tunnel.
Tests:
- _use_tmp_tunnel_file helper now also neutralises _get_collection so
existing tests don't accidentally trip the new validation path when
test-order pollution leaves a real chroma backend bound
- test_closets.py::TestTunnels setup/teardown updated to monkeypatch
resolver functions instead of the removed constant; also neutralises
_get_collection for the same reason
- Three tests in test_miner.py exercising compute_topic_tunnels are
unchanged in intent — they monkeypatch the new resolvers and pass
without stubbing _get_collection because kind=topic skips validation
- New TestTunnelFileFollowsConfig and TestCreateTunnelEndpointValidation
classes cover the regression surface for both fixes
test_legacy_state_backfills_content_hash failed on test-windows because
Path.write_text without an encoding uses the system locale (cp1252 on
Windows). The em dash was written as 0x97, then read back by
diary_ingest as UTF-8 with errors=replace — round-trip produced
different bytes than the in-Python literal, so the assertion comparing
the persisted hash to sha256(text.encode(utf-8)) diverged.
Pin the write side to encoding=utf-8 so the on-disk bytes match what
diary_ingest decodes. No production change.
Address Copilot review on #925:
- Full closet rebuild whenever the content hash differs from prior
state, not only on entry-count growth. Without this, an in-place
edit (same entry count, different body) updated the drawer but
left the closet/search index stale — defeats the verbatim guarantee
at the search layer even if the drawer is correct.
- Legacy size-only skip path now records the computed content_hash
back into state so subsequent runs use the strict hash check
instead of remaining on the size-only path indefinitely.
- Test updates: typo direction in the regression test now matches the
comment (typo "Teh" → fix "The"), assertion now also checks the
closet collection reflects the edit, and a new test exercises the
legacy-state backfill path.
The skip-if-unchanged check compared byte length only, so any in-place
edit preserving total length (typo fix "teh"→"the", word swap) was
silently dropped — a verbatim-storage violation: the user's actual
words never reached the palace.
Switch the gate to sha256(text). State entries gain a "content_hash"
field; the legacy size-only path is preserved when prev_hash is missing
so a post-upgrade run does not re-ingest every untouched diary.
Closes#925
The previous revision used multiprocessing but still relied on timing
("second process waited at least N seconds") which flakes on CI where
spawn overhead eats into the hold window. Linux CI observed the second
process report a 0.088s wait — below the 0.1s threshold — even though
the lock behavior was correct; spawn was just slow enough that the
first process had nearly finished holding when the second got past
its own spawn.
Switch to effect-based verification: each worker logs its
[enter_time, exit_time] inside the critical section, and the test
asserts the two intervals are disjoint after sorting. A broken lock
would produce overlapping intervals regardless of spawn latency; a
working lock cannot.
Also removed the mp.Queue since we no longer pass timing data back.
The macOS CI job failed ``test_lock_blocks_concurrent_access`` because
``fcntl.flock`` on BSD/macOS is per-*process*, not per-FD: two threads
in the same process both acquire even when they open their own file
descriptors. The test passed on Linux (per-FD flock) and Windows
(per-FD ``msvcrt.locking``) but was never actually exercising the
lock's real contract.
``mine_lock`` is designed to serialize multi-*agent* access — i.e.,
separate processes, not threads. Switch the test to
``multiprocessing.get_context('spawn')`` with a module-level worker
(so the spawn pickles cleanly) so it:
1. reflects the actual use case (one lock per mining process);
2. passes on all three OSes without flock-semantics branching;
3. catches real regressions (a broken lock would now let both
processes through, exactly what we care about).
Hold time bumped to 0.3s and the "wait until p1 acquires" delay to
0.2s to tolerate spawn's higher startup latency on macOS/Windows.
The Windows CI job failed on:
assert '/.mempalace/state/' in str(state_path)
because Windows uses ``\`` as the path separator, so the substring
never matches. The behavior under test (state file lives outside the
diary dir, under ``~/.mempalace/state/``) is already correct on both
platforms — only the assertion was Unix-only.
Switch to ``state_path.parent`` comparisons that work on any OS.
Brings in PR #793 (optional LLM-based closet regeneration via
user-configured OpenAI-compatible endpoint) and PR #795 (hybrid
closet+drawer search — closets boost, never gate). Stack: #784 → #788
→ #789 → #790 → #791 → #792 → #793 (+ #795).
Findings hardened on our side
─────────────────────────────
1) closet_llm.regenerate_closets didn't use the blessed palace helpers.
Before:
* manual closets_col.get(where=...) + .delete(ids=...) with a
silent ``except Exception: pass`` around both — if the purge
failed, pre-existing regex closets survived alongside fresh LLM
closets, giving the searcher double hits for the same source.
* ``source.split('/')[-1][:30]`` to build the closet_id — quietly
wrong on Windows paths (``C:\\proj\\a.md`` has no ``/``, so the
whole string ends up in the ID).
* no mine_lock around purge+upsert — a concurrent regex rebuild of
the same source could interleave with our purge and leave a mix
of regex and LLM pointers.
* no ``normalize_version`` stamp on the LLM closets — the miner's
stale-version gate would treat them as leftovers from an older
schema and rebuild over them on the next mine.
After: routes through ``purge_file_closets`` + ``mine_lock`` +
``os.path.basename`` + ``NORMALIZE_VERSION`` stamp. Regression tests
cover each.
2) searcher.search_memories was still closet-first.
PR #795 merged into #793's head to fix the recall regression
documented in that PR (R@1 0.25 on narrative content vs. 0.42
baseline). The hybrid design makes closets a ranking boost rather
than a gate: drawers are always queried at the floor, and matching
closet hits (rank 0-4 within CLOSET_DISTANCE_CAP=1.5) add a boost
of 0.40/0.25/0.15/0.08/0.04 to the effective distance.
Merged to take the incoming hybrid design, with two cleanups:
* kept the ``_expand_with_neighbors`` / ``_extract_drawer_ids_from_closet``
helpers as separately-tested utilities (still imported by tests
and future callers);
* replaced the fragile ``source_file.endswith(basename)`` reverse-
lookup in the enrichment step with internal ``_source_file_full``
/ ``_chunk_index`` fields stripped before return, so enrichment
doesn't silently pick the wrong path when two sources share a
basename across directories;
* drawer-grep enrichment now sorts by ``chunk_index`` before
neighbor expansion, so ``best_idx ± 1`` corresponds to actual
document order rather than whatever order Chroma returned.
3) Closet-first tests in test_closets.py (``TestSearchMemoriesClosetFirst``,
end-to-end ``test_closet_first_search_includes_drawer_index_and_total``)
pinned contracts that the hybrid path now violates (``matched_via``
went from ``"closet"`` to ``"drawer+closet"``). Rewrote them around
the new invariant: direct drawers are always the floor, closet
agreement flips the hit's matched_via and exposes closet_preview.
Verification
────────────
* 805/805 pass under ``uv run pytest tests/ -v --ignore=tests/benchmarks``
(13 new tests from PR #793 + 5 from PR #795 + 2 new regressions for
the closet_llm hardening + the rewritten hybrid assertions in
test_closets.py).
* CI-pinned ruff 0.4.x clean on ``mempalace/`` + ``tests/`` (check +
format both pass).
* No new deps — closet_llm.py still uses stdlib ``urllib.request`` per
the PR's "zero new dependencies" promise.
Co-Authored-By: MSL <232237854+milla-jovovich@users.noreply.github.com>
Merges the full hardened stack (#788 closets, #789 entity/BM25/diary,
#790 tunnels) and reimplements the drawer-grep feature in a way that
composes with the chunk-level closet-first search instead of fighting it.
## Background
The original PR added "drawer-grep" on top of the pre-hardening closet
code that returned whole-file blobs. My #788 hardening changed that
path to return *chunk-level* hits by parsing each closet's
``→drawer_id`` pointers and hydrating exactly those drawers. That made
the original drawer-grep grep-over-all-drawers logic redundant — the
closet already points at the relevant chunk.
What remained valuable from the original PR was the *context expansion*
idea: a chunk boundary can clip a thought mid-stride (matched chunk
says "here's a breakdown:" and the breakdown lives in the next chunk),
so callers want ±1 neighbor chunks for free rather than a follow-up
get_drawer call.
## Change
New ``_expand_with_neighbors(drawers_col, doc, meta, radius=1)`` helper
in searcher.py:
* Reads ``source_file`` + ``chunk_index`` from the matched drawer's
metadata.
* Fetches the ±radius sibling chunks in a SINGLE ChromaDB query using
``$and + $in`` — no "fetch all drawers for source" blowup.
* Sorts retrieved chunks by chunk_index, joins with ``\n\n``.
* Does a cheap metadata-only second query to compute ``total_drawers``
so callers know where in the file they landed.
* Graceful fallback to the matched doc alone on any ChromaDB failure or
missing metadata — search never breaks because expansion failed.
``_closet_first_hits`` now calls this helper and tags each hit with
``drawer_index`` + ``total_drawers``. Hit shape stays consistent with
the direct-search path (both still carry ``matched_via``) so callers
can't tell which path produced a given hit except via that field.
## Tests
6 new cases in TestDrawerGrepExpansion:
* neighbors returned in chunk_index order (not hash order)
* edge case: matched chunk at index 0 — only next neighbor surfaces
* edge case: matched chunk at last index — only prev neighbor surfaces
* edge case: 1-drawer file — returns just the matched doc
* missing/non-int chunk_index metadata — graceful fallback
* end-to-end via ``search_memories`` — closet-first hit carries
drawer_index, total_drawers, and includes ±1 neighbors
761/761 suite pass; ruff + format clean on CI-pinned 0.4.x.
Merge resolutions: miner.py kept develop's purge+NORMALIZE_VERSION;
searcher.py dropped the old whole-file-blob block entirely in favor of
rebuilding context expansion on top of ``_closet_first_hits``;
test_closets.py took develop's 47-test baseline and appended
TestDrawerGrepExpansion.
Merges the hardened closet/entity/BM25/diary stack from #789 and fixes
five correctness/durability issues in the tunnels module plus the
directional/symmetric design question.
## Design: tunnels are now symmetric
Per review discussion: a tunnel represents "these two things relate",
not "A causes B". The canonical ID now hashes the *sorted* endpoint
pair, so ``create_tunnel(A, B)`` and ``create_tunnel(B, A)`` resolve to
the same record and the second call updates the label rather than
creating a duplicate. ``follow_tunnels`` can be called from either
endpoint and surfaces the other side consistently.
The returned dict still preserves ``source``/``target`` in the order
the caller supplied, so UIs that want to render the connection
directionally can do so.
## Correctness fixes
* **Atomic write** — ``_save_tunnels`` writes to ``tunnels.json.tmp``
and ``os.replace``s it into place. A crash mid-write can no longer
leave a truncated file that silently reads back as ``[]`` and wipes
every tunnel. Includes ``f.flush() + os.fsync`` before replace on
platforms that support it.
* **Concurrent-write lock** — ``create_tunnel`` and ``delete_tunnel``
wrap the load→mutate→save cycle in ``mine_lock(_TUNNEL_FILE)``.
Without this, two agents creating tunnels simultaneously would both
read the same snapshot and the later writer would drop the earlier
writer's tunnel.
* **Corrupt-file tolerance** — ``_load_tunnels`` now uses a context
manager, validates that the loaded JSON is a list, and returns ``[]``
for any read failure. Subsequent ``create_tunnel`` then overwrites
the corrupt file via atomic write — no manual recovery needed.
* **Input validation** — new ``_require_name`` helper rejects empty or
whitespace-only wing/room names with a clear ``ValueError``. Prevents
phantom tunnels with blank endpoints from ever reaching the JSON
store.
* **Timezone-aware timestamps** — ``created_at`` / ``updated_at`` now
use ``datetime.now(timezone.utc).isoformat()``, matching diary ingest
and other recent modules.
## Tests (12 in TestTunnels)
5 original + 7 regression cases:
* ``test_tunnel_is_symmetric`` — A↔B and B↔A dedupe to one record.
* ``test_follow_tunnels_works_from_either_endpoint`` — symmetric surface.
* ``test_empty_endpoint_fields_rejected`` — validation guard.
* ``test_corrupt_tunnel_file_does_not_lose_new_writes`` — truncated
JSON treated as empty; next create persists cleanly.
* ``test_atomic_write_leaves_no_stray_tmp_file`` — no leftover ``.tmp``.
* ``test_concurrent_creates_preserve_all_tunnels`` — 5 threads each
create a distinct tunnel; all 5 persisted (regression for the
read-modify-write race).
* ``test_created_at_is_timezone_aware`` — ISO8601 has tz suffix.
Merge resolutions: tests/test_closets.py combined develop's hardened
closet/entity/BM25/diary tests with this PR's TestTunnels class.
755/755 tests pass. ruff + format clean under CI-pinned 0.4.x.
Merges develop (closet hardening #826, strip_noise #785, lock #784) and
replaces every sub-feature in this PR with a correct, tested
implementation. Shippable now.
## 1. Real Okapi-BM25 (searcher.py)
The prior `_bm25_score()` hardcoded `idf = log(2.0)` for every term — it
was really a scaled TF, not BM25, and couldn't tell a discriminative
term from a generic one. Replaced with `_bm25_scores(query, documents)`
that computes proper IDF over the provided candidate corpus using the
Lucene smoothed formula `log((N - df + 0.5) / (df + 0.5) + 1)`. Well-
defined for re-ranking vector-retrieval candidates — IDF there measures
how discriminative each term is *within the candidate set*, exactly the
signal we want.
`_hybrid_rank` also fixed:
- Vector normalization is now absolute `max(0, 1 - dist)`, not
`1 - dist/max_dist` — adding/removing a candidate no longer reshuffles
the others.
- BM25 is min-max normalized within candidates (bounded [0, 1]).
- Closet path now re-ranks too (was previously returning closet-order
hits without hybrid scoring).
- `_hybrid_score` internal field stripped from output; `bm25_score`
exposed for debugging.
## 2. Entity metadata (miner.py)
- Reuses `_ENTITY_STOPLIST` from palace.py so sentence-starters like
"When", "After", "The" no longer land as entities (regression test
covers this).
- Known-entity registry is cached at module level, keyed by the
registry file's mtime — no more disk read per drawer.
- File handle now uses a context manager.
- Truncates the entity LIST (to 25) before joining — never splits a
name in the middle.
## 3. Diary ingest (diary_ingest.py)
- State file now lives at `~/.mempalace/state/diary_ingest_<hash>.json`,
keyed by (palace_path, diary_dir). No more pollution of the user's
content directory.
- Drawer IDs now hash `(wing, date_str)` — a user with personal + work
diaries on the same day no longer silently clobbers.
- Each day's upsert runs inside `mine_lock(source_file)` so concurrent
ingest from two terminals can't race.
- `force=True` now calls `purge_file_closets` before rebuild so
leftover numbered closets from a longer prior day don't orphan.
## 4. Tests (tests/test_closets.py)
Merged this PR's MineLock/Entity/BM25/Diary tests with develop's
hardened Build/Upsert/Purge/Rebuild/SearchClosetFirst tests. Added
specific regression tests for every fix above:
- entity stoplist applies (no "When/After/The")
- entity list capped before join (no partial tokens)
- registry cached by mtime (mock-verified zero re-reads)
- BM25 IDF downweights terms present in every doc (real BM25 evidence)
- hybrid rank absolute normalization stable against outliers
- diary state file outside user's diary dir
- diary wing-prefixed IDs prevent cross-wing date collisions
35/35 closet tests pass; full suite 743/743. ruff + format clean under
CI-pinned 0.4.x.
Merges develop (#820 version sync, #785 strip_noise + NORMALIZE_VERSION,
#784 file locking) and addresses six concerns surfaced during PR review
of the closet feature:
1. Closet append-on-rebuild bug — upsert_closet_lines used to APPEND to
existing closets (mismatched the doc's "fully replaced" promise). With
NORMALIZE_VERSION rebuilds on develop, this would have stacked stale
v1 topics on top of fresh v2 content forever. Fix:
- Drop the read-and-append branch from upsert_closet_lines (now a pure
numbered-id overwrite).
- Add purge_file_closets(closets_col, source_file) helper that wipes
every closet for a source file by where-filter.
- process_file calls purge_file_closets before upsert on every mine,
mirroring the existing drawer purge.
2. Searcher returned whole-file blobs from the closet path while the
direct path returned chunk-level drawers. Refactored:
- _extract_drawer_ids_from_closet parses the `→drawer_a,drawer_b`
pointers out of closet documents.
- _closet_first_hits hydrates exactly those drawer IDs (chunk-level),
not collection.get(where=source_file) (which returned everything).
- Same hit shape as direct-search path; both now carry matched_via.
3. max_distance was bypassed on the closet path. Now applied per-hit;
when every closet candidate gets filtered, _closet_first_hits returns
None and the caller falls through to direct drawer search.
4. Entity extraction caught sentence-starters like "When", "The",
"After" as proper nouns. Added _ENTITY_STOPLIST (~40 common false
positives + day/month names + role words). Real names like Igor /
Milla still survive — covered by tests.
5. CLOSETS.md drifted from the code (claimed "replaced via upsert" but
code appended; claimed BM25 hybrid that doesn't exist; claimed a
10K char hydration cap that wasn't enforced). Rewritten to describe
what actually ships, with explicit notes on the BM25 / convo-closet
follow-ups.
6. Zero tests for ~250 lines. Added tests/test_closets.py with 17 cases:
- build_closet_lines: pointer shape, header extraction, stoplist
filtering (with regression case for "When/After/The"), real-name
survival, fallback-line guarantee, drawer-ref slicing.
- upsert_closet_lines: pure overwrite semantics (regression for the
append bug), char-limit packing without splitting lines.
- purge_file_closets: scoped to source_file, doesn't touch others.
- End-to-end miner rebuild: re-mining a file with fewer topics fully
purges leftover numbered closets from the larger first run.
- _extract_drawer_ids_from_closet: parsing + dedup edge cases.
- search_memories closet-first: fallback when empty, chunk-level
hits with matched_via, no whole-file glue, max_distance enforced.
Merge resolutions: miner.py imports combined NORMALIZE_VERSION/mine_lock
from develop with the closet helpers from this branch. process_file
auto-merged cleanly (closet block sits inside develop's lock body).
724/724 tests pass. ruff + format clean under CI-pinned 0.4.x.
Appended from Milla's omnibus test_closets.py — covers create,
list, delete, dedup, and follow_tunnels behavior. 21/21 pass.
Co-Authored-By: MSL <232237854+milla-jovovich@users.noreply.github.com>
Trimmed version of Milla's omnibus test_closets.py to only cover
features present in this PR stack (#784 lock, #788 closets, this
PR's entity/BM25/diary). Strip-noise tests will land with #785;
tunnel tests will land with the tunnels PR.
16/16 pass.
Co-Authored-By: MSL <232237854+milla-jovovich@users.noreply.github.com>
Igor Lins e Silva
mvalentsev
Milla J
yonefive71