Harden Chroma startup preflight and repair recovery - #1
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f21f9cf0cb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
946c49d to
c7e7819
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c7e7819d00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74cf1e592b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94e5301002
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR hardens MemPalace’s Chroma repair flow and backend startup preflight to better tolerate corrupt/stale on-disk state, and adds targeted regression coverage around these repair paths.
Changes:
- Add a temp-collection rebuild strategy with count verification and clearer failure/rollback signaling (
RebuildCollectionErrorwithlive_replaced). - Add a startup preflight that quarantines invalid persisted HNSW metadata before
PersistentClientopens. - Expand repair-path tests across backend, CLI, and repair module flows (success + failure modes).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
mempalace/repair.py |
Refactors extraction and rebuild into helpers; rebuild now stages via a temp collection and verifies counts; improved failure reporting/rollback behavior. |
mempalace/cli.py |
Updates mempalace repair command to reuse new repair helpers and temp-rebuild flow. |
mempalace/backends/chroma.py |
Adds invalid-HNSW-metadata quarantine and wires it into client creation preflight. |
tests/test_backends.py |
Adds coverage for invalid-metadata quarantine and client preflight ordering / cold-start behavior. |
tests/test_cli.py |
Updates CLI repair test to assert the new temp-rebuild and upsert behavior. |
tests/test_repair.py |
Adds/updates repair-module rebuild tests for temp staging and failure scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d51a040cf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c21c38623
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39fcb2fd14
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7ee4a8c09d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5b7b8d71b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e83f2a04f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4293d83360
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5395c4b6bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4196ecfe7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
mjc
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mjc
changed the title
The MCP `mempalace_get_drawer` tool returned the entire raw drawer metadata blob to any connected client, and the `source_file` field in that blob is the absolute filesystem path written by the miners (`miner.py`, `convo_miner.py` — `source_file = str(filepath)`). On a single-user local deployment this is self-disclosure, but in nested-agent or multi-server MCP topologies the client is a separate trust domain and the host's directory layout has no documented client-side use. Mirror the mitigation that `searcher.search_memories()` already applies on its own return path: reduce `source_file` to its basename via `Path(source_file).name` before handing the metadata to the client. Citations still work — the directory layout does not leak. Companion to #1 (omit palace_path from tool_status). Same threat class, different surface: - mempalace_status — palace dir path → fixed in #1 - mempalace_get_drawer — per-drawer source_file path → this PR Other read tools were audited and do not leak host paths: - mempalace_search — already basenames source_file - mempalace_list_drawers — returns wing/room/preview only - mempalace_diary_read — date/timestamp/topic/content only - mempalace_reconnect — success/message/drawers only - mempalace_kg_* — entity/predicate strings, counts - mempalace_check_duplicate — wing/room/preview only Changes: - mempalace/mcp_server.py: tool_get_drawer() now basenames metadata.source_file - tests/test_mcp_server.py: regression test asserting the absolute path and its parent directory do not appear anywhere in the response - website/reference/mcp-tools.md: clarify the documented return shape
Adds _try_gemini_json parser to normalize.py for three layouts:
1. Gemini API contents format (~/.gemini/sessions/*.json):
{"contents": [{"role": "user", "parts": [{"text": "..."}]}, ...]}
2. Messages-wrapper variant:
{"messages": [{"role": "user", ...}, {"role": "model", ...}]}
3. Flat top-level list with role="model".
This complements the existing _try_gemini_jsonl parser (which handles
~/.gemini/tmp/<hash>/chats/session-*.jsonl with session_metadata
sentinel) — JSONL covers Gemini CLI runtime sessions, JSON covers
exported / Studio-saved transcripts.
## Review feedback addressed (PR MemPalace#204)
bgauryy review:
- #1 Parser-precedence bug: _try_gemini_json runs *before*
_try_claude_ai_json so the {"messages":[..., role=model, ...]}
layout is no longer silently claimed by the Claude parser. The
Gemini parser's has_model_role guard prevents false-positives
against Claude / ChatGPT data.
- #2 Layout 2a coverage: TestGeminiJson.test_messages_wrapper_format
+ test_messages_wrapper_does_not_get_claimed_by_claude pin the
fix in place.
- #3 Test conflicts with current main: rebased onto develop;
tests restructured into TestGeminiJson class.
- #4 tempfile/os.unlink → pytest tmp_path everywhere.
- #5 elif not text → else (the elif branch was dead).
- MemPalace#6 Module docstring updated to mention Google AI Studio.
Tests: 9 new cases in TestGeminiJson covering all three layouts,
multi-part text joining, non-text part skipping, has_model_role
disambiguation, dispatch-chain regression for review #1.
…ier 6a Igor's review on PR MemPalace#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.
Summary
Relationship to upstream MemPalace#1124
Upstream PR MemPalace#1124 introduced the original stale-HNSW quarantine path as part of a broader Windows/status hardening change. This PR is the focused repair/startup follow-up for that Chroma work: it keeps the cold-start quarantine idea, tightens when destructive quarantine runs, adds invalid-metadata preflight coverage, and hardens the repair/rollback paths that recover from quarantined or corrupted HNSW state.
This PR intentionally does not carry MemPalace#1124's unrelated Windows CLI encoding or status pagination changes.
Commit stack
fix(chroma): harden HNSW startup preflightfix(repair): rebuild collections through temp stagingfix(cli): restore backup on repair failureTesting
ruff check mempalace/backends/chroma.py mempalace/cli.py mempalace/repair.py tests/test_backends.py tests/test_cli.py tests/test_repair.pypython -m pytest tests/test_backends.py tests/test_cli.py tests/test_repair.py -q(153 passed)Related PRs