Merge pull request #253 from arabold/copilot/fix-delete-docs-bug

fix: delete pages before versions to prevent foreign key constraint failure

Author: arabold

package-lock.json

@@ -155,6 +155,82 @@ describe("DocumentStore - With Embeddings", () => {
       expect(await store.checkDocumentExists("templib", "1.0.0")).toBe(false);
     });
 
+    it("should completely remove a version including pages and documents", async () => {
+      const docs: Document[] = [
+        {
+          pageContent: "First document for removal test",
+          metadata: {
+            title: "Doc 1",
+            url: "https://example.com/doc1",
+            path: ["docs"],
+          },
+        },
+        {
+          pageContent: "Second document for removal test",
+          metadata: {
+            title: "Doc 2",
+            url: "https://example.com/doc2",
+            path: ["docs"],
+          },
+        },
+      ];
+
+      // Add documents and verify they exist
+      await store.addDocuments("removelib", "1.0.0", docs);
+      expect(await store.checkDocumentExists("removelib", "1.0.0")).toBe(true);
+
+      // Remove the version
+      const result = await store.removeVersion("removelib", "1.0.0", true);
+
+      // Verify the results
+      expect(result.documentsDeleted).toBe(2);
+      expect(result.versionDeleted).toBe(true);
+      expect(result.libraryDeleted).toBe(true);
+
+      // Verify documents no longer exist
+      expect(await store.checkDocumentExists("removelib", "1.0.0")).toBe(false);
+    });
+
+    it("should remove version but keep library when other versions exist", async () => {
+      const v1Docs: Document[] = [
+        {
+          pageContent: "Version 1 document",
+          metadata: {
+            title: "V1 Doc",
+            url: "https://example.com/v1",
+            path: ["v1"],
+          },
+        },
+      ];
+
+      const v2Docs: Document[] = [
+        {
+          pageContent: "Version 2 document",
+          metadata: {
+            title: "V2 Doc",
+            url: "https://example.com/v2",
+            path: ["v2"],
+          },
+        },
+      ];
+
+      // Add two versions
+      await store.addDocuments("multilib", "1.0.0", v1Docs);
+      await store.addDocuments("multilib", "2.0.0", v2Docs);
+
+      // Remove only version 1.0.0
+      const result = await store.removeVersion("multilib", "1.0.0", true);
+
+      // Verify version 1 was deleted but library remains
+      expect(result.documentsDeleted).toBe(1);
+      expect(result.versionDeleted).toBe(true);
+      expect(result.libraryDeleted).toBe(false);
+
+      // Verify version 1 no longer exists but version 2 does
+      expect(await store.checkDocumentExists("multilib", "1.0.0")).toBe(false);
+      expect(await store.checkDocumentExists("multilib", "2.0.0")).toBe(true);
+    });
+
     it("should handle multiple versions of the same library", async () => {
       const v1Docs: Document[] = [
         {

src/store/DocumentStore.test.ts

@@ -80,6 +80,7 @@ export class DocumentStore {
     getPageId: Database.Statement<[number, string]>;
     deleteDocuments: Database.Statement<[string, string]>;
     deleteDocumentsByUrl: Database.Statement<[string, string, string]>;
+    deletePages: Database.Statement<[string, string]>;
     queryVersions: Database.Statement<[string]>;
     checkExists: Database.Statement<[string, string]>;
     queryLibraryVersions: Database.Statement<[]>;
@@ -238,6 +239,14 @@ export class DocumentStore {
            WHERE p.url = ? AND l.name = ? AND COALESCE(v.name, '') = COALESCE(?, '')
          )`,
       ),
+      deletePages: this.db.prepare<[string, string]>(
+        `DELETE FROM pages 
+         WHERE version_id IN (
+           SELECT v.id FROM versions v
+           JOIN libraries l ON v.library_id = l.id
+           WHERE l.name = ? AND COALESCE(v.name, '') = COALESCE(?, '')
+         )`,
+      ),
       getDocumentBySort: this.db.prepare<[string, string]>(
         `SELECT d.id
          FROM documents d
@@ -1093,9 +1102,18 @@ export class DocumentStore {
 
       const { id: versionId, library_id: libraryId } = versionResult;
 
+      // Delete in order to respect foreign key constraints:
+      // 1. documents (page_id → pages.id)
+      // 2. pages (version_id → versions.id)
+      // 3. versions (library_id → libraries.id)
+      // 4. libraries (if empty)
+
       // Delete all documents for this version
       const documentsDeleted = await this.deleteDocuments(library, version);
 
+      // Delete all pages for this version (must be done after documents, before version)
+      this.statements.deletePages.run(normalizedLibrary, normalizedVersion);
+
       // Delete the version record
       const versionDeleteResult = this.statements.deleteVersionById.run(versionId);
       const versionDeleted = versionDeleteResult.changes > 0;