chore: sync upstream/develop + record #1459 + #1474 merges - #60
Merged
Conversation
Two functions construct the metadatas[] list that gets fed to
chromadb's upsert/add, both vulnerable to the same ValueError:
ValueError: Expected metadata to be a non-empty dict, got 0
metadata attributes in add.
chromadb 1.5.x's validate_metadata rejects both `None` and `{}`
entries — see chromadb/api/types.py:validate_metadata (line ~1071).
This commit patches both:
1. `_extract_drawers` (line ~131) — the chromadb-collection-based
extractor, used when the source palace's collection is openable
via the chromadb client. Sanitizes None/{} entries in the
`batch["metadatas"]` list to `{"_repaired_empty_meta": True}`
before extending `all_metas`.
2. `_rebuild_one_collection` (line ~813) — the SQLite-direct
extract path used by `rebuild_from_sqlite()`, invoked when the
source palace can't be opened via chromadb (the recovery path
for palaces with corrupt HNSW segments). Old code was:
metas.append(meta if meta else {})
The trailing `{}` was the bug; chromadb 1.5.x rejects empty
dicts the same as None. Replaced with the same sentinel.
Why `_repaired_empty_meta: True` as the sentinel:
- Satisfies chromadb's non-empty-dict requirement
- Bool-valued (valid chromadb metadata type, trivially serializable)
- Namespaced + descriptive so an operator can find which drawers
were coerced via `where={"_repaired_empty_meta": True}` later
- Idempotent on re-runs (a future repair over a sanitized palace
sees the sentinel as already-valid)
Verified on a 151,478-drawer production palace that previously
crashed at drawer 120,000 in both extract paths.
Fixes MemPalace#1458
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Belt-and-suspenders on top of the repair.py sanitization in the
previous commit. A 151,478-drawer rebuild test still failed at
~120K with the same `ValueError: Expected metadata to be a
non-empty dict` from chromadb, even with the repair.py sanitizers
in place. Traceback:
mempalace/backends/chroma.py:add → chromadb Collection.add
→ validate_insert_record_set → validate_metadatas
→ validate_metadata → ValueError
Likely cause: chromadb's `upsert()` internally calls `add()` for
new records, and somewhere between repair.py's batch upsert and
chromadb's final write, the metadatas list gets reprocessed in a
way that re-introduces empty/None entries.
Sanitizing at the chromadb-client chokepoint catches everything:
no caller can leak bad metadata regardless of upstream sanitization
state. Same `{"_repaired_empty_meta": True}` sentinel, searchable
via `where={"_repaired_empty_meta": True}`.
Cost: one list comprehension per write call; negligible vs the
embedding + HNSW work each upsert already does.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ct_drawers Addresses @Copilot's review feedback on MemPalace#1459. Five tests: - test_extract_drawers_preserves_valid_metadata: non-empty dict passes through unchanged (regression guard against breaking happy path). - test_extract_drawers_sanitizes_none_metadata: None entries coerce to {"_repaired_empty_meta": True} (the core fix). - test_extract_drawers_sanitizes_empty_dict_metadata: empty dict {} entries also coerce to the sentinel (chromadb 1.5.x rejects both shapes equally). - test_extract_drawers_sanitization_preserves_alignment: critical invariant — ids[i] / documents[i] / metadatas[i] stay in lockstep through the sanitizer; mis-pairing would silently corrupt rebuilds. - test_extract_drawers_multiple_batches: pagination boundary correctness (sanitizer applied per-batch, no drops/duplicates). Verified passing locally against mempalace fork main + chromadb 1.5.8 in the palace-daemon venv (5 passed, 67 deselected in 1.88s). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…E queries
mine_convos was calling file_already_mined() once per file inside the
main loop. On a 150k-drawer palace, each per-file query
(`collection.get(where={"source_file": X}, limit=1)`) costs ~2 seconds
because chromadb has to scan the metadata index. A 2000-transcript
directory took >1h of wall-clock just to decide every file should be
skipped — and pegged multiple cores doing so, starving the daemon's
other endpoints.
bulk_check_mined() already existed for exactly this anti-pattern (its
docstring says "Callers can check membership and compare mtimes locally
instead of issuing one ChromaDB query per file") but only the project
miner used it; the convo miner kept the slow per-file path.
This patch adds a third helper, prefetch_mined_set(), that mirrors
file_already_mined()'s version-gate semantics (the check_mtime=False
branch used by mine_convos) and returns a set[str] for O(1) lookups.
mine_convos now calls it once before the loop; the loop body becomes
a set-membership check.
Observed on a 172k-drawer palace probing 10 files:
before: 21.2s (2.12s/file)
after: single bulk pass should be 30–60s for the whole 172k scan,
then 2000 O(1) checks ≈ free
file_already_mined() is kept for callers that genuinely need the
per-file semantics (the post-lock race-check in _file_chunks_locked
at convo_miner.py:350 still uses it intentionally).
Surfaced via jphein/familiar.realm.watch foundation-rework debugging
on 2026-05-11. Originally filed as #51.
…ppet Pass the GA Measurement ID from the GitHub Actions repo variable into the docs build so the published site at mempalaceofficial.com actually emits the gtag tags. Also escape the ID via encodeURIComponent / JSON.stringify so a malformed value can't break the page.
feat(docs): wire Google Analytics into the published docs site
fix(repair): coerce empty metadata to sentinel during rebuild
…os-bulk-prefetch perf(convo_miner): bulk pre-fetch already-mined set instead of N WHERE queries
…op-and-doc-updates-2026-05-12
… open-PR queue Two upstream PRs from this fork merged today (2026-05-12 UTC): - MemPalace#1459: empty-metadata sentinel during repair rebuild - MemPalace#1474: convo_miner bulk pre-fetch (the bulk_check_mined() landing) YAML changes: - repair-empty-meta-sentinel: pr_state OPEN → MERGED - new entry convo-miner-bulk-prefetch-already-mined (commit 248854a, pr 1474, pr_state MERGED) at top of entries list per newest-first convention. Documents the helper as the Row 1 fork-ahead item from the original CLAUDE.md inventory, finally upstreamed. FORK_CHANGELOG.md regenerated via scripts/render-docs.py. README "Open upstream PRs": - Drop MemPalace#1459 row - Add MemPalace#1484 row (OpenCode source adapter on RFC 002, filed today, co-authored with @JakobSachs) - Update date 2026-05-11 → 2026-05-12 - Add "Two merged today" note linking MemPalace#1459 + MemPalace#1474 Includes upstream/develop merge through commit 2d6c0bf (7 upstream commits brought into fork main, 3 auto-merges on chroma.py / convo_miner.py / palace.py, no conflicts). scripts/check-docs.sh: 4/4 clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
… ETA
Previously rebuild_index() printed "Staged N/M" / "Re-filed N/M" lines
with no time information, leaving operators to do mental math against
wall-clock to estimate completion on a large palace. On a 183k-drawer
rebuild, it's the difference between "this is a 20-min job" and "this
is an 8-hour job" — non-trivial scheduling information that the
function already has and could easily share.
This patch:
1. Adds a `progress: Optional[Callable[[str], None]] = None` parameter
to `rebuild_index()`. Defaults to a new `_DefaultProgress` class
when omitted (backward-compatible default behavior — same content
plus ETA decoration).
2. `_DefaultProgress.__call__` recognizes `Staged N/M` and `Re-filed N/M`
lines via regex, computes elapsed/rate/ETA, and appends them:
Staged 5000/182953 drawers... (elapsed 7m, rate 11.3/s, ETA 4h)
Non-progress lines (e.g. "Backing up chroma.sqlite3...") pass
through unchanged.
3. The clock + baseline counter reset at the stage→refile transition
so refile-phase rate isn't muddied by the slower stage phase
(refile re-embeds the same drawers and may run at different
throughput).
4. Replaces the in-function `print(...)` status calls with `progress(...)`
so a custom callable receives the full status stream — not just the
batch progress lines.
5. Passes `progress=progress` (instead of hardcoded `progress=print`)
into `_rebuild_collection_via_temp`.
Use cases for a custom `progress` callable:
- Daemon-side capture for HTTP `/repair/status` (this is the motivating
case — palace-daemon was tailing the output to surface progress to
operators via the status endpoint; a callback avoids stdout capture
trickery)
- Test silence: `progress=lambda *_: None`
- Custom formatting: structured logging, JSON event stream, etc.
The default behavior is unchanged for callers that omit the parameter;
they just get a nicer message format.
Closes MemPalace#1485.
…ndex-progress-callback feat(repair): rebuild_index accepts progress callback; default prints ETA
…op-and-doc-updates-2026-05-12
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
upstream/developinto forkmain(7 commits brought in; 3 auto-merges onchroma.py,convo_miner.py,palace.py; no conflicts)docs/fork-changes.yaml: `repair-empty-meta-sentinel` (PR fix(repair): coerce empty metadata to sentinel during rebuild MemPalace/mempalace#1459) → `pr_state: MERGED`248854a, PR perf(convo_miner): bulk pre-fetch already-mined set instead of N WHERE queries MemPalace/mempalace#1474, MERGED) — Row 1 of the original CLAUDE.md fork-ahead inventory, finally upstreamedFORK_CHANGELOG.mdviascripts/render-docs.pyTest plan
scripts/check-docs.shpasses 4/4 (test count skip, 21/21 commit hashes resolve, FORK_CHANGELOG matches YAML, 77/77 PR states match)upstream/developwas a clean 3-way (auto-merge, no manual resolution)🤖 Generated with Claude Code