fix(index): update and delete spatial and vector index entries (#164)

[CaffeinatedCoder]

Fixes #164.

Problem

CollectionSecondaryIndex.Update and Delete only maintained B-Tree indexes (_btreeIndex?.). DynamicCollection.IndexDelete skipped vector and spatial indexes outright. Neither RTreeIndex nor VectorSearchIndex could remove an entry. A moved or deleted document stayed findable at its old position or embedding, and once its slot was reused, searches returned the unrelated document stored there.

Fix

RTreeIndex.Delete(GeoBox, DocumentLocation)

• Walks only the subtrees whose box covers the point and removes the matching leaf entry.
• Shrinks the bounding boxes above it with the existing UpdateMBRUpwards.
• A node left empty stays in the tree instead of being freed. Its box is GeoBox.Empty, which matches no search. Freeing would need a transactional page free; a rollback would otherwise leave the parent pointing at a freed page.

VectorSearchIndex.Delete(float[], DocumentLocation)

• Marks the node deleted by overwriting its stored document location with default. No document lives on page 0.
• The node keeps its vector and links, so the graph stays connected. Search skips deleted nodes and fills the top-k from the rest of its candidate list.
• To find the node, it first searches the graph with the old vector, which normally finds the node at distance 0. It falls back to scanning the node pages only if that search misses.
• Writes go through the transaction, so a rollback restores the node.
• No page-format change: indexes written by older versions read the same.

VectorSearchIndex.Relocate(float[], DocumentLocation, DocumentLocation) re-points the node of a document that moved without changing its vector. DynamicCollection updates always move the document to a new slot. Without this, every update would leave a deleted node behind.

CollectionSecondaryIndex routes insert, update and delete through per-type helpers. Vectors are now compared by value when deciding whether an update touches the index. Every load creates a new array, so reference equality would have rewritten the HNSW graph on every update of any field.

DynamicCollection deletes vector and spatial entries. Its update paths use a new IndexUpdate helper that relocates an unchanged vector.

Delete and Relocate are new public members on the public VectorSearchIndex class.

Tests

SpatialVectorIndexMaintenanceTests, 12 tests. 10 fail on main:

• The four repros from the issue: spatial update, spatial delete then insert elsewhere, vector update, vector delete then insert elsewhere.
• Deleting half of 1,000 points spread over many R-Tree nodes, then reusing the freed slots elsewhere: Within returns exactly the survivors.
• A relocating update (the document outgrows its slot) moves the spatial entry.
• Deleting 30 of 40 vectors: search returns exactly the 10 survivors in order.
• Updates that leave the vector unchanged, including a relocating one, keep one result per document.
• DynamicCollection: spatial update/delete, and vector update/delete/rename.

The other two (spatial and vector delete rolled back) guard the new transactional deletes; on main they pass because delete did nothing.

The existing HnswGraphQualityTests (recall) still pass. Full suite on macOS: 2391 passed, 3 failed, 4 skipped. The 3 failures are the known MultiProcessWalSharedMemoryTests .wal-shm lock timeouts.

Limitations

• Deleted HNSW nodes still take part in graph traversal. In an index with many deletes close to a query, the search candidate list can run short of live nodes, so fewer than k results may come back. Rebuilding the index drops them: VacuumAsync(RebuildIndexes = true) once fix(vacuum): rebuild typed-collection indexes into fresh trees (#167) #170 is in, or dropping and recreating the index.
• Emptied R-Tree nodes are not freed or condensed until a rebuild. R-Tree pages still have no CollectAllPages (unchanged).
• The indexed Near / non-indexed Near / Within correctness issues from Near / Within always return true outside the index: non-indexed spatial queries match every document, and indexed Near returns the whole bounding box #165 are separate.

🤖 Generated with Claude Code

[mrdevrobot]

Thanks, this is careful work: the R-Tree delete (boundary points, exact (mbr, PageId, SlotIndex) match, MBR shrink via CalculateMBR, GeoBox.Empty never intersecting), the tombstone choice (page 0 is the header, never a document page), the by-value vector comparison and the transactional writes all check out, and the page format is unchanged. 73/73 in the targeted tests, full suite green apart from the known .wal-shm failures, netstandard2.1 builds, no conflicts with main (5.2.3). One blocker.

1. HNSW Search returns fewer than k, down to zero, live results after deletes (VectorSearchIndex.cs:482-503).
SearchLayer(..., Math.Max(efSearch, k), 0) fills its result heap with the ef nearest nodes including tombstones, and only afterwards the loop skips loc.PageId == 0. Nothing over-fetches or keeps exploring. Repro (DynamicCollection, L2, 3 dims): insert v0..v299 at [i,i,i], delete v0..v199, query [0,0,0] with k=10 → 0 results, expected v200..v209. Vector_DeleteMost_SearchReturnsOnlySurvivors passes only because ef=100 ≥ 40 nodes, so it never exercises this. Fix as hnswlib does: keep expanding through deleted nodes in SearchLayer (they stay in visited/candidates for routing) but only push live nodes into the result heap, so ef counts live results. Please bump the test to more nodes than efSearch (e.g. 300 inserted / 200 deleted).

2. Tombstones grow without a reclaim path on typed collections. A relocating update on DocumentCollection is DeleteCore + InsertIntoAll (DocumentCollection.cs:2812-2829), so every update that outgrows its slot tombstones a node even when the vector is unchanged; Relocate is only used by DynamicCollection. Please route the typed relocating update through Relocate as well, and state in the docs that a rebuild (once #170 lands) is the way to drop tombstones.

3. RewriteLocation fallback is a full scan of the node pages (VectorSearchIndex.cs:566-577), hit deterministically when more than EfConstruction documents share an identical vector (zero/default embeddings are common). Not a correctness issue; a comment or a cap would help.

Nits: Delete/Relocate return bool but every caller discards it (a debug log when the entry is missing would help); CollectionSecondaryIndex.Delete still decrements _documentCount when nothing was found (pre-existing); vector Search/RewriteLocation run without the index-level lock that RTreeIndex takes, consistent with Insert but worth a comment.

Posted from the wrong account by mistake; the maintainer review from @mrdevrobot supersedes this.

[CaffeinatedCoder]

Thanks for the repro. Addressed in the latest commit; the branch also merges main (5.2.3).

1. Fewer than k results. SearchLayer now has a query mode, used by Search, that works like hnswlib:

• Deleted nodes are still expanded and stay in visited/candidates for routing, but only live nodes enter the result heap.
• The stop condition is d_c > bound && result.Count >= ef, so the search keeps going until it holds ef live nodes or runs out of candidates.
• Graph construction and RewriteLocation keep the old behaviour, where deleted nodes are valid neighbours.

The test is now Vector_DeleteMoreThanEfSearch_SearchStillReturnsKLiveResults: your scenario, 300 inserted and 200 deleted, all closer to the query than any survivor. It returns exactly v200..v209 and fails on the previous commit. The existing HNSW recall tests still pass.

2. Tombstones on typed relocating updates.

• UpdateAtLocationCore's relocating branch now calls DeleteCore(..., deleteFromIndexes: false) and then UpdateInAll(old, new, oldLocation, newLocation).
• CollectionSecondaryIndex.Update is location-aware: an unchanged key at a new location is re-pointed. For a vector index that means Relocate, with delete + insert as the fallback if a node is missing. B-Tree and spatial entries are deleted and reinserted at the new location.
• New test Vector_RelocatingUpdatesWithAnUnchangedVector_AddNoNodes counts the HNSW nodes on disk across three relocating updates. On the previous commit it grows by one node per update; now it doesn't grow.

Rebuilding the index with VacuumAsync(RebuildIndexes = true), once #170 lands, is the way to drop accumulated tombstones; the XML docs of Delete already say so.

3. RewriteLocation fallback. It now has a comment saying when the page scan runs: more than EfConstruction documents sharing one vector, e.g. all-zero embeddings. I didn't add a cap. A capped scan could fail to find the node and fall back to delete + insert, which costs a tombstone; that seemed the worse trade. Happy to change it if you prefer the cap.

Nits:

• The missing index-level lock in Search/RewriteLocation is now commented: callers serialize writes through the collection lock, as with Insert.
• The discarded bool from Delete/Relocate: there's no logging facility in Core to report a missing entry. Update now uses Relocate's result to fall back to delete + insert.
• The _documentCount decrement on a missed delete is pre-existing, and I left it alone here.

Full suite on macOS: 2405 passed, 0 failed, 4 skipped.

🤖 Generated with Claude Code

[mrdevrobot]

All three points are fixed and both new tests fail with their fixes reverted. The query mode matches hnswlib: Bound() is infinite until a live node is in the result, the upper-layer descent still routes through deleted nodes, and construction and RewriteLocation are unchanged. I ran a recall probe with 2,000 random 16-dim vectors, k = 10, 50 queries against brute force over the survivors:

deleted	recall@10 now	recall@10 before	ms/query now
50%	1.000	1.000	1.4
90%	1.000	0.918	2.1
99%	1.000	0.094	3.1

The relocating update path is correct for B-Tree, spatial and vector indexes, and the other DeleteCore callers keep the default. Full suite 2405 / 0 / 4 skipped. Approving.

Small follow-ups:

• The XML doc on CollectionSecondaryIndex.Update (~L111-121) still says relocating callers must use DeleteFromAll + InsertInAll and that the locations must match. It's now wrong.
• Relocating updates no longer adjust _documentCount when a key goes from null to non-null or back. The in-place path already had this gap; statistics only.
• Query cost now grows with tombstones when fewer than ef live nodes are reachable. That's expected; a sentence in the Delete docs pointing at the rebuild would help.

[CaffeinatedCoder]

Thanks. Addressed in 58d9c71.

• Update XML doc: rewritten for the three cases: the same key and location keeps the entry, the same key at a new location re-points it, and otherwise it deletes and inserts. newLocation may differ from oldLocation.
• _documentCount: fixed on both the in-place and the relocating path. Update now increments when the key goes from null to non-null and decrements on the reverse, as Insert/Delete do. Test: Update_BetweenNullAndNonNullKey_AdjustsEstimatedDocumentCount (in place and relocating). It fails without the change.
  • Side note: DocumentCollection.GetIndexes() caches its CollectionIndexInfo list until the set of indexes changes, so the EstimatedDocumentCount it returns goes stale after the first call either way. The test reads GetInfo() directly. I can file that separately if you want.
• Delete docs: they now say that search keeps expanding deleted nodes until it holds ef live ones, so with fewer than ef reachable live nodes the query cost grows with the tombstones, and that VacuumOptions.RebuildIndexes removes them.

[mrdevrobot]

Verified 58d9c71: the Update doc, the null ↔ non-null _documentCount adjustment and the Delete docs are all correct, and the commit changes nothing else. On main with #170 merged, a vacuum rebuild does drop tombstones: 100 nodes after insert, 100 after deleting 60, 40 after VacuumAsync(RebuildIndexes = true), still 40 after reopen. Full suite on the merged state 2425 / 0 / 4 skipped.

One low-severity follow-up for an issue, not blocking: with several vectors on one key, all &= Relocate(...) re-points every vector it can; if one fails, the fallback deletes at the old location (missing the re-pointed ones) and re-inserts all of them, leaving duplicate live nodes (CollectionSecondaryIndex.cs:142-145, :227-233). It needs an already inconsistent index to trigger.

Approving and merging.