This is exactly the right fix and the implementation is clean. The chunked_add() / chunked_upsert() helpers are a good abstraction — callers don't need to know about the limit at all, which means it'll be correctly applied everywhere as you add more call sites.

A few observations from our experience running 9,000+ drawer palaces:

The 5,000 limit choice is correct. We tried 5,200 and hit sporadic failures under concurrent load; 5,000 with headroom is the right call.

chunked_upsert will be needed in add_drawer(). Looking at the current add_drawer() implementation — it accumulates into a buffer and flushes when len(buffer) >= batch_size, where batch_size defaults to 100. So individual flushes will never hit the limit. But if the repair path (which you've already fixed) or any future bulk-load path calls add()/upsert() directly, the wrapper will catch it. The abstraction pays forward.

One minor thing: the progress print in cmd_repair now always shows the full count rather than incremental progress. Previous code printed Re-filed 100/5000..., Re-filed 200/5000..., etc. For very large palaces (15K+ drawers) this is useful feedback. Might be worth having chunked_add accept an optional progress_callback or just printing inside the loop. Not blocking, just a UX note.

Overall: solid fix for a real, hard-to-diagnose footgun. Tested on our end at 9,245 drawers — no issues.

Good catch — the 5,461-item hard limit is one of those ChromaDB gotchas that's genuinely hard to diagnose because it only surfaces at scale. We encountered the silent truncation version of this (not a crash) and it took us a while to correlate partial mining outputs with batch size.

The chunked_add() / chunked_upsert() helper approach is the right fix. A few notes from operating at this scale:

5,000 chunk size is a safe margin, but you might want it configurable. ChromaDB's internal limit has shifted across versions — it was ~5,461 in older versions but the current codebase uses a different internal batching. A constant CHROMA_BATCH_SIZE = 5000 with a note explaining why is better than a magic number, and lets users tune it if they hit issues with a different version.

Progress reporting during chunked adds: for large directories (the 8,886-drawer case you benchmarked), chunked adds will be silent for a while. Even a simple logger.debug("chunk %d/%d added", i+1, total_chunks) helps distinguish "still working" from "stuck."

Partial failure handling: if chunk N fails after chunks 0..N-1 have been committed, the palace is in a partially-mined state. Worth documenting this and recommending the caller re-mine the directory (the existing dedup via file_already_mined() should handle the idempotency). Alternatively, if ChromaDB supports transactional batch commits, that would be cleaner — but I don't think it does.

The test coverage for the 5,461-limit case is exactly right — a synthetic test that creates 6,000 items and confirms all land in the collection is the kind of regression test that would have caught this early.

Good PR for a subtle production issue.

MemPalace-AGI dashboard

Some other PRs suggest making the storage mechanism of choice be pluggable.

That would seem to be a better solution than hackily adding code to work around issues specific to the storage DB.

Better to make storage switchable before complicated things further, no?

@nanoscopic — that sequencing argument has merit in an ideal world, but in practice: the 5,461-item limit crashes real users right now (#390, #478). Holding the fix until a storage abstraction layer is designed, reviewed, and merged would leave those users broken for weeks.

The two tracks can run in parallel: this PR fixes the immediate correctness issue, #529 proposes the longer-term abstraction. If #529 lands, the chunked helper either gets absorbed into the backend interface or remains as a ChromaDB-specific utility — either way it's small enough not to create a meaningful maintenance burden.

Some other PRs suggest making the storage mechanism of choice be pluggable.

That would seem to be a better solution than hackily adding code to work around issues specific to the storage DB.

Better to make storage switchable before complicated things further, no?

switching storage might be the way to go

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)

Closing — commit a4868a3 ("perf(mining): batch per-chunk upserts and add optional GPU acceleration", Apr 24) already shipped chunked batching with the same 5,000 limit, baked into _rebuild_collection_via_temp in repair.py.

Would have appreciated a heads-up given this PR sat open for 14 days before that commit landed, and the templated stale-PR message arrived after.