From d08bffd5fec4ab54ef3f843813a32061c026af98 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= <ricardo@mr-gross.de>
Date: Tue, 13 Oct 2026 15:38:20 +0200
Subject: [PATCH] docs(spatial): address review of R-tree page enumeration
 (#189)

- VacuumOptions.RebuildIndexes: drop the stale "Spatial indexes are not
  rebuilt" sentence; spatial indexes are now rebuilt like the others.
- RTreeIndex.CollectAllPages: document that the walk does not take the
  index lock and which lock each caller holds.
- RTreeIndex.CollectAllPages: Debug.Fail when a page is reached twice
  (cycle or shared child); the seen set still keeps release builds finite.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 src/BLite.Core/Indexing/RTreeIndex.cs | 17 ++++++++++++++++-
 src/BLite.Core/VacuumOptions.cs       |  2 +-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/src/BLite.Core/Indexing/RTreeIndex.cs b/src/BLite.Core/Indexing/RTreeIndex.cs
index 2acea28..8cdd333 100644
--- a/src/BLite.Core/Indexing/RTreeIndex.cs
+++ b/src/BLite.Core/Indexing/RTreeIndex.cs
@@ -3,6 +3,7 @@
 using BLite.Core.Collections;
 using BLite.Core.Indexing.Internal;
 using System.Buffers;
+using System.Diagnostics;
 
 namespace BLite.Core.Indexing;
 
@@ -121,6 +122,12 @@ public IEnumerable<DocumentLocation> SearchNear(double latitude, double longitud
     /// Returns all physical page IDs owned by this R-tree.
     /// Used when dropping or rebuilding an index to free storage space.
     /// </summary>
+    /// <remarks>
+    /// The walk does not take this index's lock. Callers must hold the lock that excludes writers
+    /// to the tree: the collection write lock during <c>VacuumAsync</c>'s rebuild, the index
+    /// manager's lock (which every index insert, update and delete also takes) when an index is
+    /// dropped, and a dropped collection is disposed before its pages are collected.
+    /// </remarks>
     public IReadOnlyList<uint> CollectAllPages() => CollectAllPages(0);
 
     /// <summary>
@@ -142,7 +149,15 @@ internal IReadOnlyList<uint> CollectAllPages(ulong transactionId)
             while (stack.Count > 0)
             {
                 uint pageId = stack.Pop();
-                if (pageId == 0 || !seen.Add(pageId)) continue;
+                if (pageId == 0) continue;
+                if (!seen.Add(pageId))
+                {
+                    // Every node has exactly one parent, so reaching a page twice means a cycle or a
+                    // shared child. The set keeps the walk finite and the page counted once, but the
+                    // tree is corrupt.
+                    Debug.Fail($"R-tree page {pageId} is reachable more than once from root {_rootPageId}.");
+                    continue;
+                }
                 pages.Add(pageId);
 
                 _storage.ReadPage(pageId, transactionId == 0 ? null : transactionId, buffer);
diff --git a/src/BLite.Core/VacuumOptions.cs b/src/BLite.Core/VacuumOptions.cs
index 57ee779..5aa8add 100644
--- a/src/BLite.Core/VacuumOptions.cs
+++ b/src/BLite.Core/VacuumOptions.cs
@@ -41,7 +41,7 @@ public sealed class VacuumOptions
     /// the existing index is kept. Index handles obtained earlier (for example from
     /// <c>GetIndexAsync</c> or <c>CreateIndexAsync</c>) refer to the replaced tree afterwards and
     /// must be fetched again. A query already running on the old tree while it is replaced may read
-    /// pages that are being freed. Spatial indexes are not rebuilt.
+    /// pages that are being freed. Spatial (R-tree) indexes are rebuilt the same way.
     /// </para>
     /// </summary>
     public bool RebuildIndexes { get; init; } = false;