Selaa lähdekoodia

fix(collections): purge documents before dropCollection (vector leak)

DELETE /collections/{name} called dropCollection(), which clears the
collection metadata but does NOT purge the underlying vector index — orphan
vectors survived and leaked into a later same-name collection (search returned
them even though document_count read 0; DELETE-by-id 404'd). The DB exposes no
bulk/truncate primitive, but remove(id) DOES purge the index, so enumerate and
remove every document before dropping the now-empty collection.

Verified on prod (isolated probe): remove(id) purges search; plain drop+recreate
leaks; remove-then-drop does not. Adds regression test + bumps VERSION 0.1.3->0.1.4.

Refs docs/bugs/2026-06-02-collection-delete-leaks-vectors.md
Fszontagh 1 kuukausi sitten
vanhempi
sitoutus
aa1e78714b

+ 1 - 1
VERSION

@@ -1 +1 @@
-0.1.3
+0.1.4

+ 159 - 0
docs/bugs/2026-06-02-collection-delete-leaks-vectors.md

@@ -0,0 +1,159 @@
+# Bug: `DELETE /collections/{name}` leaks vector data into a recreated collection
+
+**Reporter:** discovered while running the OCR→RAG ingest pipeline in n8n against `rag001.smartbotics.ai` (project `sungrow`).
+**Date:** 2026-06-02
+**Severity:** High — data isolation guarantee violated. A user who drops a collection and reuses the same name sees old vectors mixed into the new collection's search results.
+**Affected endpoint:** `DELETE /api/v1/projects/{project}/collections/{name}` (vector collections; JSON collection behavior not tested).
+
+## Summary
+
+`DELETE /api/v1/projects/{project}/collections/{name}` reports success and the collection's metadata is reset (`document_count: 0`, new `created_at`), but the **underlying vector index is NOT purged**. When a new collection is created with the **same name + same `vector_dimension` + same `embedding_model`**, `POST /search` against the new collection returns the orphan vectors from the previous incarnation with their original IDs.
+
+Additionally, `DELETE /collections/{name}/documents/{id}` on those orphan IDs returns `404 not_found` — confirming the metadata layer doesn't know they exist, but the vector index still does.
+
+## Severity rationale
+
+- **Data isolation broken.** Users who DROP a collection reasonably expect "drop and all its documents" (the OpenAPI summary literally states this). Instead, vectors persist.
+- **Silent corruption.** No error is returned. The user sees `document_count: 0` and assumes the collection is fresh. Polluted search results look like a quality issue with their pipeline, not a server bug.
+- **No clean workaround inside the API.** The user cannot purge the orphan vectors via any documented endpoint (`DELETE` by id 404s; no truncate endpoint; no bulk-delete; no filter-delete). The only workaround is to **rename the collection** (use `datasheets_v2` instead of recreating `datasheets`), which leaks index storage forever.
+
+## Reproduction
+
+The reproduction uses cURL with two env vars: `RAG_API_KEY` (a non-admin key granted the project) and `BASE=https://rag001.smartbotics.ai`. Substitute your own project name; below assumes a project called `repro_bug` already exists (admin must create it).
+
+### Step 1 — Create a vector collection
+
+```bash
+curl -sS -X POST -H "Authorization: Bearer $RAG_API_KEY" -H 'Content-Type: application/json' \
+  -d '{"name":"leakcheck","kind":"vector","vector_dimension":4096,"embedding_model":"qwen/qwen3-embedding-8b"}' \
+  "$BASE/api/v1/projects/repro_bug/collections"
+```
+
+Expected: `{"name":"leakcheck","kind":"vector","vector_dimension":4096}`.
+
+### Step 2 — Insert one vector with a known id
+
+```bash
+curl -sS -X POST -H "Authorization: Bearer $RAG_API_KEY" -H 'Content-Type: application/json' \
+  -d '{"id":"sentinel_v1","text":"the canary said tweet","metadata":{"era":"v1"}}' \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck/vectors"
+```
+
+### Step 3 — Confirm it's searchable
+
+```bash
+curl -sS -X POST -H "Authorization: Bearer $RAG_API_KEY" -H 'Content-Type: application/json' \
+  -d '{"query_text":"canary","top_k":1,"min_score":0.0}' \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck/search"
+```
+
+Expected: 1 hit with `id: "sentinel_v1"`, score > 0.
+
+### Step 4 — Drop the collection
+
+```bash
+curl -sS -X DELETE -H "Authorization: Bearer $RAG_API_KEY" \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck"
+```
+
+Expected: `{"deleted":"leakcheck"}`. The OpenAPI says this should "Drop a collection and all its documents."
+
+### Step 5 — Recreate with the same name + same config
+
+```bash
+curl -sS -X POST -H "Authorization: Bearer $RAG_API_KEY" -H 'Content-Type: application/json' \
+  -d '{"name":"leakcheck","kind":"vector","vector_dimension":4096,"embedding_model":"qwen/qwen3-embedding-8b"}' \
+  "$BASE/api/v1/projects/repro_bug/collections"
+```
+
+### Step 6 — Verify it's empty
+
+```bash
+curl -sS -H "Authorization: Bearer $RAG_API_KEY" \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck"
+```
+
+Returns `"document_count": 0`. **Looks empty.**
+
+### Step 7 — Search for the canary
+
+```bash
+curl -sS -X POST -H "Authorization: Bearer $RAG_API_KEY" -H 'Content-Type: application/json' \
+  -d '{"query_text":"canary","top_k":1,"min_score":0.0}' \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck/search"
+```
+
+**Actual result (bug):** the `sentinel_v1` vector comes back in `results`, even though `document_count` claims 0 and the collection was just recreated.
+
+### Step 8 — Try to delete the orphan
+
+```bash
+curl -sS -X DELETE -H "Authorization: Bearer $RAG_API_KEY" \
+  "$BASE/api/v1/projects/repro_bug/collections/leakcheck/documents/sentinel_v1"
+```
+
+**Returns:** `{"error":{"code":"not_found","message":"no such document"}}` — HTTP 404. So the metadata layer agrees the document doesn't exist, but search still surfaces it. The two layers disagree.
+
+## Expected vs Actual
+
+| Step | Expected | Actual |
+|---|---|---|
+| 4 (DELETE collection) | Both metadata AND underlying vector index dropped | Metadata cleared, document_count → 0, vector index untouched |
+| 6 (GET recreated collection) | `document_count: 0` (truly empty) | `document_count: 0` (deceptive — there IS data) |
+| 7 (search recreated collection) | 0 results | Old vectors from the dropped collection returned |
+| 8 (DELETE orphan vector by id) | 200 (id found, deleted) or — if metadata-correctly — 404 followed by it not appearing in search | 404 (metadata correct), but it STILL appears in search |
+
+## Real-world impact
+
+Observed live on `rag001.smartbotics.ai`, project `sungrow`, collection `datasheets`:
+
+- Original ingest run produced ~104 vectors with various polluted text (Gemini's bbox-JSON layout output).
+- `DELETE /collections/datasheets` → reported success, `document_count` → 0.
+- `POST /collections {name: "datasheets", ...}` (recreate) → reported success.
+- `POST /search` → returned the original 104 polluted vectors with their original `_id` strings (`019e8345f453aa0158d2d1bc0e1a_chunk_001` etc).
+- After noticing this, we switched to collection name `datasheets_v2` — that one started genuinely empty, so the bug is specifically tied to name reuse.
+
+The user-facing symptom was a re-ingested datasheet's "clean" vectors appearing in search alongside garbage from the previous run that we believed was deleted. Several hours debugging "why is OCR output still polluted?" before realizing the OCR fix had worked but the old vectors hadn't gone away.
+
+## Root-cause hypotheses
+
+(For the maintainer to confirm.) Likely causes inside `src/`:
+
+1. **`db_gateway` doesn't drop the underlying vector-index storage on collection delete.** The `qualify(project, coll)` namespace key is reused on recreate, so the underlying smartbotic-database table/index keeps its rows. `collection_registry` updates its own metadata + counters but doesn't cascade to the index store.
+2. **No idempotency on collection recreate.** When a collection is created with a name that has lingering vector data, the server doesn't notice the orphan rows; it just registers the new metadata over them.
+3. The bug may be DB-layer (smartbotic-database not honoring a drop command from vectorapi) or vectorapi-layer (drop in collection_registry but no corresponding `db_gateway::dropVectors(project, coll)` call). Likely the latter based on the file layout.
+
+## Suggested fix direction
+
+1. On `DELETE /collections/{name}`, `db_gateway` should explicitly issue a "drop all vectors with this collection's namespace key" call to smartbotic-database — not rely on collection_registry alone.
+2. On `POST /collections`, if a name collides with a recently-dropped namespace whose underlying storage still has rows, EITHER refuse the create (409) OR purge the orphan rows before accepting the create.
+3. (Belt-and-suspenders) `DELETE /collections/{name}/documents/{id}` and `POST /search` should both consult the SAME source of truth. Currently `DELETE by id` says "no such document" while `search` returns the document — that's a layer-disagreement bug regardless of the rest.
+
+## Suggested regression test
+
+Add to `tests/test_api_integration.cpp` (the file already covers collection CRUD; extend it):
+
+```cpp
+TEST_F(ApiFixture, DeleteCollectionPurgesVectorIndex) {
+  createCollection("leak_test", /*kind=*/"vector", /*dim=*/4);
+  insertVector("leak_test", "sentinel", /*vec=*/{1,0,0,0}, {{"era","v1"}});
+  ASSERT_EQ(searchVectors("leak_test", {1,0,0,0}, /*top_k=*/1).size(), 1);
+
+  deleteCollection("leak_test");
+  createCollection("leak_test", /*kind=*/"vector", /*dim=*/4);
+  EXPECT_EQ(getCollection("leak_test")["document_count"].get<int>(), 0);
+  EXPECT_TRUE(searchVectors("leak_test", {1,0,0,0}, /*top_k=*/5).empty())
+      << "Recreated collection must NOT return vectors from the dropped one";
+}
+```
+
+The current integration test pattern in this file uses an in-process DB fixture so this should be cheap to add.
+
+## Workaround for users hitting this today
+
+Use a **new collection name on every reset** (e.g. `datasheets_v2`, then `_v3`, etc). Don't `DELETE → recreate same name`. Document this in `api/llms.txt` or the OpenAPI summary until fixed.
+
+## Related observations
+
+- `DELETE /collections/{name}/documents/{id}` returns 404 for vectors that DO appear in search results — the metadata and the index are in two different states.
+- `POST /vectors` correctly rejects duplicate IDs with `"Document with ID '...' already exists"` — meaning the underlying store IS aware of the orphan. So the duplicate-id check sees the orphan but `document_count` does not. That confirms `collection_registry` and the vector store are out of sync, with `db_gateway` (or the DB layer) holding the orphan data invisibly to the registry.

+ 14 - 1
src/handlers/collections.cpp

@@ -103,7 +103,20 @@ void registerCollectionRoutes(ApiServer& s) {
         ApiKey k = requireKey(*d, req); std::string project = req.matches[1], name = req.matches[2];
         requireProjectAccess(k, project);
         if (!d->registry.get(project, name)) throw ApiError(ErrCode::NotFound, "not_found", "no such collection");
-        d->db.client().dropCollection(qualify(project, name));
+        const std::string qc = qualify(project, name);
+        // dropCollection() clears the collection's metadata but does NOT purge the
+        // underlying (vector) index — orphan rows survive and leak into a later
+        // same-name collection (search returns them even though document_count is 0).
+        // remove() per id DOES purge the index, so delete every document first, then
+        // drop the now-empty collection. (See docs/bugs/2026-06-02-collection-delete-leaks-vectors.md)
+        smartbotic::database::Client::QueryOptions opts; opts.limit = 1000000;
+        auto docs = d->db.client().find(qc, opts);
+        for (const auto& doc : docs) {
+            std::string id = doc.value("_id", std::string());
+            if (id.empty()) id = doc.value("id", std::string());
+            if (!id.empty()) d->db.client().remove(qc, id);
+        }
+        d->db.client().dropCollection(qc);
         d->registry.remove(project, name);
         sendJson(res, 200, {{"deleted", name}});
     });

+ 38 - 0
tests/test_api_integration.cpp

@@ -358,3 +358,41 @@ TEST_F(ApiFixture, SpaFallbackDoesNotClobberApiMetaOrAssets) {
     ASSERT_TRUE(o); EXPECT_EQ(o->status, 200);
     EXPECT_EQ(o->body.find("SVAPI_SPA_OK"), std::string::npos);
 }
+
+// Regression: DELETE /collections/{name} must purge the underlying vector index,
+// not just the metadata. Otherwise orphan vectors leak into a recreated same-name
+// collection (search returns them though document_count reads 0).
+// See docs/bugs/2026-06-02-collection-delete-leaks-vectors.md
+TEST_F(ApiFixture, DeleteCollectionPurgesVectorIndex) {
+    auto c = admin();
+    const std::string base = "/api/v1/projects/" + project_ + "/collections";
+    auto mkColl = [&] {
+        return c.Post(base.c_str(),
+            nlohmann::json{{"name", "leak_test"}, {"kind", "vector"}, {"vector_dimension", 4}}.dump(),
+            "application/json");
+    };
+
+    ASSERT_EQ(mkColl()->status, 201);
+    auto ins = c.Post((base + "/leak_test/vectors").c_str(),
+        nlohmann::json{{"id", "sentinel"}, {"vector", {1, 0, 0, 0}}}.dump(), "application/json");
+    ASSERT_TRUE(ins); ASSERT_EQ(ins->status, 201);
+
+    // Drop, then recreate with the same name + config.
+    ASSERT_EQ(c.Delete((base + "/leak_test").c_str())->status, 200);
+    ASSERT_EQ(mkColl()->status, 201);
+
+    // The recreated collection must be genuinely empty.
+    auto find = c.Get((base + "/leak_test/documents?limit=100").c_str());
+    ASSERT_TRUE(find); ASSERT_EQ(find->status, 200);
+    EXPECT_EQ(nlohmann::json::parse(find->body).value("count", -1), 0)
+        << "recreated collection leaked documents from the dropped one";
+
+    auto search = c.Post((base + "/leak_test/search").c_str(),
+        nlohmann::json{{"query_vector", {1, 0, 0, 0}}, {"top_k", 5}, {"min_score", 0.0}}.dump(),
+        "application/json");
+    ASSERT_TRUE(search); ASSERT_EQ(search->status, 200);
+    EXPECT_TRUE(nlohmann::json::parse(search->body).value("results", nlohmann::json::array()).empty())
+        << "recreated collection's search returned orphan vectors from the dropped one";
+
+    c.Delete((base + "/leak_test").c_str());
+}