Merge branch 'main' into feat/update-delete-benchmark-refactor

Author: danielebarbaro

src/BM25/Index.php

@@ -38,6 +38,13 @@ final class Index
     /** @var array<int, Document> nodeId → Document */
     private array $documents = [];
 
+    /**
+     * Per-document term list (unique terms only).
+     * Enables O(|terms in doc|) deletion instead of O(|vocabulary|).
+     * @var array<int, string[]>
+     */
+    private array $docTerms = [];
+
     public function __construct(
         private readonly Config $config = new Config(),
         private readonly TokenizerInterface $tokenizer = new SimpleTokenizer(),
@@ -73,6 +80,9 @@ public function addDocument(int $nodeId, Document $document): void
         foreach ($termFreqs as $term => $tf) {
             $this->invertedIndex[$term][$nodeId] = $tf;
         }
+
+        // Track which terms this document contributed so removal is O(|terms in doc|).
+        $this->docTerms[$nodeId] = array_keys($termFreqs);
     }
 
     /**
@@ -210,15 +220,15 @@ public function removeDocument(int $nodeId): bool
             unset($this->docLengths[$nodeId]);
         }
 
-        // Remove from inverted index.
-        foreach ($this->invertedIndex as $term => &$postings) {
-            unset($postings[$nodeId]);
+        // Remove from inverted index — only touch terms this document contained.
+        foreach ($this->docTerms[$nodeId] ?? [] as $term) {
+            unset($this->invertedIndex[$term][$nodeId]);
             // Remove empty posting lists to save memory.
-            if (empty($postings)) {
+            if (empty($this->invertedIndex[$term])) {
                 unset($this->invertedIndex[$term]);
             }
         }
-        unset($postings);
+        unset($this->docTerms[$nodeId]);
 
         unset($this->documents[$nodeId]);
 
@@ -237,7 +247,8 @@ public function vocabularySize(): int
      * @return array{
      *   totalTokens: int,
      *   docLengths: array<int, int>,
-     *   invertedIndex: array<string, array<int, int>>
+     *   invertedIndex: array<string, array<int, int>>,
+     *   docTerms: array<int, string[]>
      * }
      */
     public function exportState(): array
@@ -246,6 +257,7 @@ public function exportState(): array
             'totalTokens'   => $this->totalTokens,
             'docLengths'    => $this->docLengths,
             'invertedIndex' => $this->invertedIndex,
+            'docTerms'      => $this->docTerms,
         ];
     }
 
@@ -256,7 +268,8 @@ public function exportState(): array
      * @param array{
      *   totalTokens: int,
      *   docLengths: array<int, int>,
-     *   invertedIndex: array<string, array<int, int>>
+     *   invertedIndex: array<string, array<int, int>>,
+     *   docTerms?: array<int, string[]>
      * } $state
      * @param array<int, Document> $documents  nodeId → Document (from HNSW index)
      */
@@ -266,5 +279,18 @@ public function importState(array $state, array $documents): void
         $this->docLengths    = $state['docLengths'];
         $this->invertedIndex = $state['invertedIndex'];
         $this->documents     = $documents;
+
+        // Rebuild docTerms from the inverted index when loading older snapshots
+        // that were persisted before this field was introduced.
+        if (isset($state['docTerms'])) {
+            $this->docTerms = $state['docTerms'];
+        } else {
+            $this->docTerms = [];
+            foreach ($this->invertedIndex as $term => $postings) {
+                foreach (array_keys($postings) as $nId) {
+                    $this->docTerms[$nId][] = $term;
+                }
+            }
+        }
     }
 }

src/HNSW/Index.php

@@ -361,16 +361,31 @@ public function search(array $query, int $k = 10, ?int $ef = null): array
             [$epDist, $ep] = $this->searchLayerGreedy($qv, $ep, $epDist, $lc);
         }
 
-        // Full beam search at layer 0.
-        $W = $this->searchLayer($qv, [[$epDist, $ep]], $ef, 0);
+        // Full beam search at layer 0, retrying with a larger ef when soft-deleted
+        // nodes shrink the active result set below $k.  Doubling ef on each retry
+        // costs at most O(log(totalNodes / ef)) extra passes in the worst case.
+        $currentEf  = $ef;
+        $totalNodes = count($this->nodes);
+
+        do {
+            $W = $this->searchLayer($qv, [[$epDist, $ep]], $currentEf, 0);
+
+            // Filter out soft-deleted nodes.
+            if (!empty($this->deleted)) {
+                $W = array_values(array_filter(
+                    $W,
+                    fn(array $pair) => !isset($this->deleted[$pair[1]])
+                ));
+            }
 
-        // Filter out soft-deleted nodes and take the k nearest.
-        if (!empty($this->deleted)) {
-            $W = array_values(array_filter(
-                $W,
-                fn(array $pair) => !isset($this->deleted[$pair[1]])
-            ));
-        }
+            // Stop when we have enough active results, or ef already spans all nodes
+            // (further expansion cannot surface new candidates).
+            if (count($W) >= $k || $currentEf >= $totalNodes) {
+                break;
+            }
+
+            $currentEf = min($currentEf * 2, $totalNodes);
+        } while (true);
 
         $topK = array_slice($W, 0, $k);
         return $this->toSearchResults($topK);

src/Persistence/DocumentStore.php

@@ -20,7 +20,13 @@
  */
 final class DocumentStore
 {
-    /** @var int[] PIDs of outstanding async child processes. */
+    /**
+     * PIDs of outstanding async child processes, keyed by nodeId.
+     * Keying by nodeId lets waitForNode() drain exactly one write without
+     * blocking every other in-flight write.
+     *
+     * @var array<int, int>  nodeId → PID
+     */
     private array $pendingPids = [];
 
     public function __construct(private readonly string $docsDir) {}
@@ -59,8 +65,8 @@ public function write(
                 $this->writeSync($nodeId, $docId, $text, $metadata);
                 exit(0);
             } else {
-                // Parent: record PID and return.
-                $this->pendingPids[] = $pid;
+                // Parent: record PID keyed by nodeId and return.
+                $this->pendingPids[$nodeId] = $pid;
                 return;
             }
         }
@@ -69,6 +75,25 @@ public function write(
         $this->writeSync($nodeId, $docId, $text, $metadata);
     }
 
+    /**
+     * Block until the async write for a specific node has completed.
+     *
+     * Use this before deleting a node's file so a late child write cannot
+     * recreate {nodeId}.bin after the unlink().
+     */
+    public function waitForNode(int $nodeId): void
+    {
+        if (!isset($this->pendingPids[$nodeId])) {
+            return;
+        }
+
+        if (function_exists('pcntl_waitpid')) {
+            pcntl_waitpid($this->pendingPids[$nodeId], $status);
+        }
+
+        unset($this->pendingPids[$nodeId]);
+    }
+
     /**
      * Block until every outstanding async write has completed.
      * Must be called before index files are written (see VectorDatabase::save()).

src/VectorDatabase.php

@@ -173,7 +173,9 @@ public function addDocuments(array $documents): void
      * The document is soft-deleted from HNSW (excluded from results but kept
      * for graph connectivity) and fully removed from the BM25 index.
      *
-     * When persistence is enabled, the document file is also deleted from disk.
+     * When persistence is enabled, a tombstone marker is written immediately so
+     * the deletion survives a crash.  The physical doc file is removed during
+     * the next `save()` call, after the indexes are fully updated on disk.
      * Call `save()` afterward to persist the updated index state.
      *
      * @param string|int $id The document ID to delete.
@@ -202,17 +204,24 @@ public function deleteDocument(string|int $id): bool
         unset($this->nodeIdToDoc[$nodeId]);
         unset($this->docIdToNodeId[$id]);
 
-        // Delete document file from disk if persistence is enabled.
+        // Mark the document for physical deletion when persistence is enabled.
         if ($this->path !== null) {
-            $docFile = $this->path . '/docs/' . $nodeId . '.bin';
-            if (file_exists($docFile)) {
-                // Suppress PHP warning and handle failure explicitly to keep
-                // on-disk state consistent with in-memory indexes.
-                if (!@unlink($docFile) && file_exists($docFile)) {
-                    throw new \RuntimeException(
-                        "Failed to delete persisted document file: {$docFile}"
-                    );
-                }
+            // An async pcntl_fork child may still be writing {nodeId}.bin.
+            // Wait for it to finish so the file is fully on disk before we
+            // record the tombstone — this keeps the pair (bin + tombstone)
+            // consistent from the moment the tombstone is created.
+            $this->getDocumentStore()->waitForNode($nodeId);
+
+            // Write a tombstone instead of immediately removing the doc file.
+            // The physical removal happens in save() AFTER the index files have
+            // been updated, giving us crash-safety:
+            //   • crash before save()  → open() finds the tombstone and
+            //                            re-applies the deletion in memory.
+            //   • crash during save()  → at worst the doc file is an orphan;
+            //                            the indexes already reflect the deletion.
+            $tombstone = $this->path . '/docs/' . $nodeId . '.tombstone';
+            if (file_put_contents($tombstone, '') === false) {
+                throw new \RuntimeException("Failed to write tombstone file: {$tombstone}");
             }
         }
 
@@ -354,9 +363,11 @@ public function hybridSearch(
      *  2. `meta.json`   — distance code, dimension, nextId, docIdToNodeId.
      *  3. `hnsw.bin`    — HNSW graph (vectors + connections).
      *  4. `bm25.bin`    — BM25 inverted index.
+     *  5. Removes `docs/{n}.bin` + `docs/{n}.tombstone` for every pending deletion.
      *
      * Individual `docs/{n}.bin` files are written incrementally by `addDocument()`
-     * and are NOT re-written by this method.
+     * and are NOT re-written by this method.  Deletion of doc files is deferred
+     * to this method so the on-disk state is always consistent.
      *
      * @throws \RuntimeException if no path was configured or on I/O failure.
      */
@@ -398,6 +409,18 @@ public function save(): void
         $serializer = new IndexSerializer();
         $serializer->writeHnsw($this->path . '/hnsw.bin', $hnswState);
         $serializer->writeBm25($this->path . '/bm25.bin', $this->bm25Index->exportState());
+
+        // Now that all index files reflect the current state, it is safe to
+        // physically remove doc files for pending tombstone deletions.
+        $docsDir = $this->path . '/docs';
+        foreach (glob($docsDir . '/*.tombstone') ?: [] as $tombstoneFile) {
+            $nodeId  = (int) basename($tombstoneFile, '.tombstone');
+            $binFile = $docsDir . '/' . $nodeId . '.bin';
+            if (file_exists($binFile)) {
+                @unlink($binFile);
+            }
+            @unlink($tombstoneFile);
+        }
     }
 
     /**
@@ -484,6 +507,32 @@ public static function open(
 
         // $db->nodeIdToDoc intentionally starts EMPTY — documents are lazy-loaded.
 
+        // ── Reconcile crash-interrupted deletions ─────────────────────────
+        // A tombstone file docs/{nodeId}.tombstone is written by deleteDocument()
+        // before save() is called.  If the process crashed between those two
+        // steps the tombstone survives but the indexes were not yet updated.
+        // Re-apply the pending deletion now so the loaded state is consistent.
+        $docsDir = $path . '/docs';
+        if (is_dir($docsDir)) {
+            foreach (glob($docsDir . '/*.tombstone') ?: [] as $tombstoneFile) {
+                $nodeId = (int) basename($tombstoneFile, '.tombstone');
+
+                // Apply the deletion only when the node is still present in the
+                // loaded indexes (i.e., save() had not yet been called).
+                if (isset($nodeIdToDocId[$nodeId])) {
+                    $docId = $nodeIdToDocId[$nodeId];
+                    $db->hnswIndex->delete($nodeId);
+                    $db->bm25Index->removeDocument($nodeId);
+                    unset($db->docIdToNodeId[$docId]);
+                }
+
+                // Always clean up — covers the edge case where the process
+                // crashed after indexes were written but before file removal.
+                @unlink($docsDir . '/' . $nodeId . '.bin');
+                @unlink($tombstoneFile);
+            }
+        }
+
         return $db;
     }
 

tests/PersistenceTest.php

@@ -421,16 +421,26 @@ public function testDeletedDocumentFileIsRemoved(): void
         $db->addDocument(new Document(id: 2, vector: [0.0, 1.0], text: 'to keep'));
         $db->save();
 
-        // Verify doc files exist (do not rely on specific nodeId-to-filename mapping)
-        $beforeFiles = glob($this->tmpDir . '/docs/*.bin') ?: [];
-        self::assertCount(2, $beforeFiles);
+        // Verify doc files exist after initial save.
+        self::assertFileExists($this->tmpDir . '/docs/0.bin');
+        self::assertFileExists($this->tmpDir . '/docs/1.bin');
 
-        // Delete one document
+        // Delete document 1 (nodeId 0).
+        // Physical removal is deferred to save() for crash-safety; a tombstone
+        // is written immediately so the deletion survives a crash.
         $db->deleteDocument(1);
 
-        // One doc file should be removed immediately
-        $afterFiles = glob($this->tmpDir . '/docs/*.bin') ?: [];
-        self::assertCount(1, $afterFiles);
+        self::assertFileExists($this->tmpDir . '/docs/0.tombstone', 'Tombstone must be created by deleteDocument().');
+        self::assertFileExists($this->tmpDir . '/docs/0.bin', 'Doc file must NOT be removed before save().');
+        self::assertFileExists($this->tmpDir . '/docs/1.bin');
+
+        // After save() both the doc file and the tombstone must be gone.
+        $db->save();
+
+        self::assertFileDoesNotExist($this->tmpDir . '/docs/0.bin');
+        self::assertFileDoesNotExist($this->tmpDir . '/docs/0.tombstone');
+        self::assertFileExists($this->tmpDir . '/docs/1.bin');
+
     }
 
     public function testUpdateDocumentPersistsCorrectly(): void