fix(chroma): adopt chromadb's own HNSW write defaults

`_HNSW_BLOAT_GUARD` pinned `hnsw:batch_size=2` / `hnsw:sync_threshold=2` on every
collection this backend creates, to answer #1579. Three findings on chromadb 1.5.9
(PersistentClient, Rust bindings, single writer, zero concurrency) retire it.

**2 sits outside chroma's own declared valid range.** `hnsw_params.py:21-22`
validates both knobs as `isinstance(p, int) and p > 2`.

**The cost is write amplification, measured.** Bytes written (`/proc/self/io`) for
one mine of N records, 64-dim, `num_threads=1`, identical corpus and seed:

    N          2/2        100/1000    ratio
    10,000      47.1 MB    28.1 MB    1.67x
    20,000     127.4 MB    60.0 MB    2.12x
    40,000     390.3 MB   134.0 MB    2.91x

Two runs at N=20,000 reproduced within 0.1%. `sync_threshold` dominates — 3/3
measured identical to 2/2, and 1000/1000 identical to 100/1000, so `batch_size`
barely registers. The ratio grows with collection size, so the largest palaces pay
the most.

**It buys nothing.** A 5-record collection at 100/1000 — far below the threshold,
so no persist ever fires — reads back whole from a fresh process: `count()` and the
vector query both answer. On that artifact `link_lists.bin` is 0 bytes with no
`index_metadata.pickle`, which is #1579's trigger shape exactly, and
`quarantine_stale_hnsw()` creates no drift directory.

The knobs move into `_hnsw_creation_metadata()`, which both `create_collection`
paths now call, so per-collection tuning has one home.

Legacy `metadata=` is kept deliberately. Measured on 1.5.9: creating with
`configuration=` leaves `collection.metadata` as `None` and the sqlite
`collection_metadata` table empty, which blinds `_read_sync_threshold` and
`ChromaCollection.distance_metric` — both read that table. (`collection.configuration`
does read back under both paths; migrating those two readers to it is the
prerequisite for ever moving the writer, and belongs in its own change.)

Not claimed: that a small `sync_threshold` is a known chroma failure mode. No such
report was found in chroma's issues or docs, and chroma's own guidance runs the
other way. The Python persist path that #1579 and chroma#6975 describe does not
execute under the Rust bindings — `hnswlib` is not a dependency of this line.

Author: joshuafontany

mempalace/backends/chroma.py

@@ -160,34 +160,101 @@ def _hnsw_payload_appears_sane(seg_dir: str) -> bool:
     return ratio is None or ratio <= _HNSW_LINK_TO_DATA_MAX_RATIO
 
 
-# HNSW batch/sync thresholds applied at collection creation.
+# HNSW batch/sync thresholds applied at collection creation — chromadb's own
+# documented defaults (chromadb/api/configuration.py:263-273,
+# chromadb/api/collection_configuration.py:451-454,
+# chromadb/segment/impl/vector/hnsw_params.py:79-80, and
+# https://docs.trychroma.com/docs/collections/configure).
 #
-# chromadb's Rust HNSW segment writes index_metadata.pickle and
-# link_lists.bin only when internal counters cross both thresholds
-# (batch_size gates _apply_batch; sync_threshold gates _persist).
-# Records below both thresholds stay in memory and are lost on exit.
+# Both ran at 2 to answer #1579 (a sub-threshold mine left index_metadata.pickle
+# absent, and quarantine_stale_hnsw renamed the segment away). Three findings on
+# chromadb 1.5.9 (PersistentClient / Rust bindings, single writer) retire that:
 #
-# Previously 50k/50k to work around link_lists.bin sparse-file bloat
-# in pre-1.5.x Python chromadb (#344).  chromadb >=1.5.4 Rust bindings
-# (the minimum mempalace supports) do not exhibit that bloat; verified
-# at batch_size=2 with 20k records: link_lists.bin = 171 KB, no
-# sparse-file inflation.
+#   * 2 SITS OUTSIDE CHROMA'S OWN DECLARED VALID RANGE. hnsw_params.py:21-22
+#     validates both knobs as `isinstance(p, int) and p > 2`. Not an inequality
+#     between the two — a floor on each.
 #
-# The 50k guard caused #1579: mines under 50k drawers never triggered
-# _persist(), leaving index_metadata.pickle absent and link_lists.bin
-# empty.  quarantine_stale_hnsw then renamed the segment on every cold
-# open after a 300s mtime gap, accumulating .drift-* directories.
+#   * IT COSTS WRITE AMPLIFICATION, measured. Bytes written (/proc/self/io) for
+#     one mine of N records, 64-dim, num_threads=1, identical corpus and seed:
 #
-# Lowered to 2 (empirical Rust-side minimum for chromadb >=1.5.4; the
-# Rust bindings reject 1 with InvalidArgumentError) so any mine of 2+
-# drawers triggers a natural persist.  Existing palaces created under
-# the old 50k guard keep those thresholds in their collection metadata
-# until the user runs repair --mode from-sqlite --archive-existing.
-_HNSW_BLOAT_GUARD = {
-    "hnsw:batch_size": 2,
-    "hnsw:sync_threshold": 2,
+#         N        2/2        100/1000     ratio
+#         10,000    47.1 MB    28.1 MB     1.67x
+#         20,000   127.4 MB    60.0 MB     2.12x
+#         40,000   390.3 MB   134.0 MB     2.91x
+#
+#     Two runs at N=20,000 reproduced within 0.1%. sync_threshold dominates:
+#     3/3 measured identical to 2/2, and 1000/1000 identical to 100/1000. The
+#     ratio grows with collection size, so the cost is worst on the largest
+#     palaces.
+#
+#   * IT BUYS NOTHING. A 5-record collection at 100/1000 — far below the
+#     threshold, so no persist ever fires — reads back whole from a FRESH
+#     PROCESS (count and vector query both answer). On that artifact
+#     link_lists.bin is 0 bytes with no index_metadata.pickle, which is #1579's
+#     trigger shape exactly, and quarantine_stale_hnsw() creates no drift dir:
+#     _segment_appears_healthy reads it as never-persisted rather than as a torn
+#     persist.
+#
+# NOT claimed here: that a small sync_threshold is a known chroma failure mode.
+# No such report was found in chroma's issues or docs, and chroma's own guidance
+# runs the other way (raise sync_threshold for bulk inserts). The Python
+# persist path that #1579 and chroma#6975 describe does not execute under the
+# Rust bindings at all — hnswlib is not a dependency of this line.
+#
+# A palace keeps whatever thresholds it was created under;
+# `repair --mode from-sqlite --archive-existing` re-creates it under these.
+_HNSW_WRITE_DEFAULTS = {
+    "hnsw:batch_size": 100,
+    "hnsw:sync_threshold": 1000,
 }
 
+
+def _hnsw_creation_metadata(options: Optional[dict]) -> dict:
+    """Build the ``metadata=`` dict for a fresh collection from caller options.
+
+    Centralizes the HNSW knobs so a multi-collection palace can tune each
+    collection at creation while the base keeps every config value in the
+    ``collection_metadata`` table where the divergence guard, the cosine-space
+    detector (``ChromaCollection.distance_metric``), and ``_read_sync_threshold``
+    already read it. The legacy ``metadata=`` keys are kept deliberately: the
+    modern ``configuration=`` API stores the same parameters in
+    ``configuration_json`` instead, leaving ``collection.metadata`` empty and
+    silently blinding all of that existing tooling.
+
+    Caller option -> chromadb metadata key:
+
+    * ``hnsw_space``       -> ``hnsw:space``        (default ``"cosine"``)
+    * ``num_threads``      -> ``hnsw:num_threads``  (default 1; serializes inserts)
+    * ``ef_construction``  -> ``hnsw:construction_ef``
+    * ``max_neighbors``    -> ``hnsw:M``
+    * ``sync_threshold``   -> ``hnsw:sync_threshold``
+    * ``batch_size``       -> ``hnsw:batch_size``
+
+    ``sync_threshold``/``batch_size`` default to chromadb's own values
+    (:data:`_HNSW_WRITE_DEFAULTS`), which amortize the index flush across a
+    mine. A caller tunes them per collection; a caller writing far fewer
+    records than the threshold still keeps them (the Rust writer holds the
+    sub-threshold tail durable — see :data:`_HNSW_WRITE_DEFAULTS`).
+    ``ef_construction``/``max_neighbors`` are omitted when the caller does not
+    set them, so chromadb applies its own defaults.
+    """
+    opts = options if isinstance(options, dict) else {}
+    md: dict[str, Any] = {
+        "hnsw:space": opts.get("hnsw_space", "cosine"),
+        "hnsw:num_threads": int(opts.get("num_threads", 1)),
+        **_HNSW_WRITE_DEFAULTS,
+    }
+    if "ef_construction" in opts and opts["ef_construction"] is not None:
+        md["hnsw:construction_ef"] = int(opts["ef_construction"])
+    if "max_neighbors" in opts and opts["max_neighbors"] is not None:
+        md["hnsw:M"] = int(opts["max_neighbors"])
+    if "sync_threshold" in opts and opts["sync_threshold"] is not None:
+        md["hnsw:sync_threshold"] = int(opts["sync_threshold"])
+    if "batch_size" in opts and opts["batch_size"] is not None:
+        md["hnsw:batch_size"] = int(opts["batch_size"])
+    return md
+
+
 # Below this size, data_level0.bin is too small for a meaningful HNSW graph.
 # Used by _hnsw_link_lists_is_usable_for_payload (empty link_lists is fine
 # when data is trivially small) and _missing_dimensionality_appears_recoverable
@@ -633,9 +700,10 @@ def _hnsw_element_count(palace_path: str, segment_id: str) -> Optional[int]:
 # read the collection metadata (older palaces missing the row, sqlite
 # unreadable). 2000 = 2 × chromadb's default sync_threshold of 1000.
 #
-# Why dynamic: legacy palaces may still carry ``sync_threshold = 50_000``
-# (the pre-#1579 guard), so flush-lag can grow up to 50K on those palaces.
-# New palaces use sync_threshold=2 (#1579) and flush almost immediately.
+# Why dynamic: a palace carries whatever ``sync_threshold`` it was created
+# under, and flush-lag grows to that threshold before a persist fires — up
+# to 50K on a palace created under an old large guard, and 2 on one created
+# under the small guard that #1308 traces back to.
 # A fixed 2000 floor would flag actively-written legacy palaces as
 # DIVERGED the moment their queue exceeded 10% of sqlite_count, even
 # though chromadb is behaving correctly. The floor must scale with the
@@ -2355,11 +2423,7 @@ def get_collection(
             except _ChromaNotFoundError:
                 collection = client.create_collection(
                     collection_name,
-                    metadata={
-                        "hnsw:space": hnsw_space,
-                        "hnsw:num_threads": 1,
-                        **_HNSW_BLOAT_GUARD,
-                    },
+                    metadata=_hnsw_creation_metadata(options),
                     **ef_kwargs,
                 )
             except ValueError as e:
@@ -2449,11 +2513,7 @@ def create_collection(
         ef_kwargs = {"embedding_function": ef} if ef is not None else {}
         collection = self._client(palace_path).create_collection(
             collection_name,
-            metadata={
-                "hnsw:space": hnsw_space,
-                "hnsw:num_threads": 1,
-                **_HNSW_BLOAT_GUARD,
-            },
+            metadata=_hnsw_creation_metadata({"hnsw_space": hnsw_space}),
             **ef_kwargs,
         )
         return ChromaCollection(collection, palace_path=palace_path)

mempalace/mcp_server.py

@@ -71,7 +71,7 @@
 from .backends.chroma import (  # noqa: E402
     ChromaBackend,
     ChromaCollection,
-    _HNSW_BLOAT_GUARD,
+    _HNSW_WRITE_DEFAULTS,
     _pin_hnsw_threads,
     hnsw_capacity_status,
     reset_hnsw_capacity_cache,
@@ -1176,7 +1176,7 @@ def _get_collection(create=False):
                         metadata={
                             "hnsw:space": "cosine",
                             "hnsw:num_threads": 1,
-                            **_HNSW_BLOAT_GUARD,
+                            **_HNSW_WRITE_DEFAULTS,
                         },
                         **ef_kwargs,
                     )

tests/test_backends.py

@@ -523,13 +523,12 @@ def test_chroma_backend_creates_collection_with_cosine_distance(tmp_path):
     assert col.metadata.get("hnsw:space") == "cosine"
 
 
-def test_chroma_backend_sets_hnsw_bloat_guard_on_creation(tmp_path):
+def test_chroma_backend_sets_hnsw_write_defaults_on_creation(tmp_path):
     """HNSW batch/sync thresholds must land on freshly-created collection metadata.
 
-    Low thresholds (2/2 per #1579) make chromadb's Rust HNSW segment
-    persist index_metadata and link_lists after any mine of 2+ drawers.
-    Asserting both keys land on the persisted metadata also covers the
-    #1161 "config silently dropped" concern at CI time.
+    That both keys land covers #1161 (config silently dropped). The VALUES guard
+    #1308: sync_threshold sets a mine's write amplification, and batch_size must
+    stay strictly below it (#2526).
     """
     palace_path = tmp_path / "palace"
 
@@ -541,35 +540,43 @@ def test_chroma_backend_sets_hnsw_bloat_guard_on_creation(tmp_path):
 
     client = chromadb.PersistentClient(path=str(palace_path))
     col = client.get_collection("mempalace_drawers")
-    assert col.metadata.get("hnsw:batch_size") == 2
-    assert col.metadata.get("hnsw:sync_threshold") == 2
+    batch = col.metadata.get("hnsw:batch_size")
+    sync = col.metadata.get("hnsw:sync_threshold")
+    assert batch == 100
+    assert sync == 1000
+    assert batch < sync, "chroma requires batch_size < sync_threshold (#2526)"
 
 
-def test_chroma_backend_create_collection_sets_hnsw_bloat_guard(tmp_path):
-    """Same guard must apply via the legacy create_collection() path."""
+def test_chroma_backend_create_collection_sets_hnsw_write_defaults(tmp_path):
+    """The same defaults must apply via the legacy create_collection() path."""
     palace_path = tmp_path / "palace"
 
     ChromaBackend().create_collection(str(palace_path), "mempalace_drawers")
 
     client = chromadb.PersistentClient(path=str(palace_path))
     col = client.get_collection("mempalace_drawers")
-    assert col.metadata.get("hnsw:batch_size") == 2
-    assert col.metadata.get("hnsw:sync_threshold") == 2
+    assert col.metadata.get("hnsw:batch_size") == 100
+    assert col.metadata.get("hnsw:sync_threshold") == 1000
 
 
-def test_sub_threshold_mine_persists_hnsw_metadata(tmp_path):
-    """Regression for #1579: small mines must persist HNSW metadata.
+def test_sub_threshold_mine_survives_reopen_and_escapes_quarantine(tmp_path):
+    """Regression for #1579, asserted as the PROPERTY, not the mechanism.
 
-    _HNSW_BLOAT_GUARD sets batch_size=2 and sync_threshold=2 so that any
-    upsert of 2+ records crosses both thresholds, triggering chromadb's
-    _apply_batch and _persist.  Without this, index_metadata and link_lists
-    stay empty and quarantine_stale_hnsw renames the segment on cold open.
+    #1579 is a durability bug: a sub-threshold mine lost its drawers when
+    quarantine_stale_hnsw renamed the segment away on cold open. So assert what
+    the user needs — the drawers read back from a FRESH backend, and quarantine
+    leaves the segment alone.
+
+    Asserting the mechanism instead (index_metadata.pickle present after 3
+    records) only holds at sync_threshold=2, which buys #1308. Under chromadb's
+    own thresholds the Rust writer keeps the tail durable WITHOUT a pickle, so
+    the pickle is rightly absent here and asserting on it would fail a healthy
+    palace.
     """
     palace_path = str(tmp_path / "palace")
     backend = ChromaBackend()
     try:
         col = backend.get_collection(palace_path, "mempalace_drawers", create=True)
-
         col.upsert(
             ids=["a", "b", "c"],
             documents=["doc a", "doc b", "doc c"],
@@ -579,34 +586,65 @@ def test_sub_threshold_mine_persists_hnsw_metadata(tmp_path):
     finally:
         backend.close()
 
-    found_healthy_segment = False
+    # Quarantine runs on cold open; stale_seconds=0.0 forces the integrity gate
+    # onto every segment regardless of mtime delta, so the gate itself is proven.
+    moved = quarantine_stale_hnsw(palace_path, stale_seconds=0.0)
+    assert moved == [], f"quarantine fired on a healthy sub-threshold segment: {moved}"
+
     for entry in (tmp_path / "palace").iterdir():
-        if not entry.is_dir() or entry.name.startswith("."):
-            continue
-        meta = entry / "index_metadata.pickle"
-        link = entry / "link_lists.bin"
-        data = entry / "data_level0.bin"
-        if data.exists() and data.stat().st_size > _HNSW_MISSING_METADATA_DATA_FLOOR:
-            assert meta.exists(), "index_metadata missing after sub-threshold upsert"
-            assert link.exists() and link.stat().st_size > 0, "link_lists empty"
+        if entry.is_dir() and not entry.name.startswith("."):
             assert _segment_appears_healthy(str(entry))
-            found_healthy_segment = True
 
-    assert found_healthy_segment, "no VECTOR segment with data found"
+    # The durability claim: a FRESH backend still finds every drawer, and the
+    # vector index still answers.
+    reopened = ChromaBackend()
+    try:
+        col = reopened.get_collection(palace_path, "mempalace_drawers")
+        assert col.count() == 3, "sub-threshold mine lost drawers across reopen"
+        hits = col.query(query_embeddings=[[0.1] * _TEST_EMBED_DIM], n_results=3)
+        assert len(hits["ids"][0]) == 3, "vector index empty after reopen"
+    finally:
+        reopened.close()
+
 
-    # stale_seconds=0.0 forces the stage-2 integrity gate (_segment_appears_healthy)
-    # to run on every segment regardless of mtime delta, proving the fix directly.
-    moved = quarantine_stale_hnsw(palace_path, stale_seconds=0.0)
-    assert moved == [], f"quarantine fired on freshly-persisted segment: {moved}"
+def test_hnsw_write_defaults_bound_index_rewrites(tmp_path):
+    """Regression for #1308: the thresholds must bound write amplification.
+
+    chromadb rewrites the ENTIRE on-disk index once sync_threshold records
+    accumulate, so a mine of N drawers costs N/sync_threshold full rewrites of a
+    multi-megabyte segment. At 2, a 26k-drawer mine fires ~13,000 — enough to
+    wedge the compactor and to tear a persist mid-pickle.
+
+    Arithmetic, not a live mine: a realistic corpus must cost rewrites in the
+    tens, never the thousands.
+    """
+    palace_path = tmp_path / "palace"
+    ChromaBackend().get_collection(
+        str(palace_path), collection_name="mempalace_drawers", create=True
+    )
+    client = chromadb.PersistentClient(path=str(palace_path))
+    col = client.get_collection("mempalace_drawers")
+
+    sync = col.metadata.get("hnsw:sync_threshold")
+    batch = col.metadata.get("hnsw:batch_size")
+
+    assert batch < sync, "chroma requires batch_size < sync_threshold (#2526)"
+
+    realistic_corpus = 26_000
+    rewrites = realistic_corpus / sync
+    assert rewrites <= 100, (
+        f"sync_threshold={sync} costs ~{rewrites:.0f} full index rewrites over a "
+        f"{realistic_corpus}-drawer mine — that is the #1308 corruption path"
+    )
 
 
 def test_single_record_upsert_not_quarantined(tmp_path):
     """A single-record upsert must not trigger quarantine.
 
-    With batch_size=2 chromadb only persists HNSW metadata after the second
-    record.  A one-record segment has no index_metadata.pickle and no
-    link_lists.bin data; _segment_appears_healthy must treat that combination
-    as sub-threshold (never persisted), not as corruption.
+    A one-record segment sits below the HNSW thresholds, so chromadb never
+    persists: no index_metadata.pickle, no link_lists.bin data.
+    _segment_appears_healthy must read that combination as sub-threshold
+    (never persisted), not as corruption.
     """
     palace_path = str(tmp_path / "palace")
     backend = ChromaBackend()
@@ -657,7 +695,7 @@ def test_get_collection_create_true_preserves_existing_metadata(tmp_path):
     backend.get_collection(palace, collection_name="mempalace_drawers", create=True)
     col = backend.get_collection(palace, collection_name="mempalace_drawers", create=True)
     assert col._collection.metadata["hnsw:space"] == "cosine"
-    assert col._collection.metadata.get("hnsw:batch_size") == 2
+    assert col._collection.metadata.get("hnsw:batch_size") == 100
 
 
 def test_fix_blob_seq_ids_converts_blobs_to_integers(tmp_path):

tests/test_hnsw_capacity.py

@@ -289,9 +289,9 @@ def test_capacity_status_quiet_for_empty_palace(tmp_path):
 def test_capacity_status_tolerates_lag_under_large_sync_threshold(tmp_path):
     """Regression for the PR #1191 / PR #1227 conflict.
 
-    Palaces created via mempalace's _HNSW_BLOAT_GUARD (sync_threshold=
-    50_000) naturally accumulate up to ~50K queued entries between
-    flushes. The pickle-vs-sqlite probe must scale its tolerance to
+    A palace carrying a large sync_threshold (50_000) naturally accumulates
+    up to ~50K queued entries between flushes. The pickle-vs-sqlite probe
+    must scale its tolerance to
     ``2 × sync_threshold`` so this expected lag is not flagged as
     corruption — otherwise vector search disables for ~80% of the
     write cycle on any actively-mined ≥100K palace.