fixing counting and logging when it happens if it happens again, delete files from database when removing from indexing

orneryd · orneryd · commit 4cf6fa640f7f · 2025-12-10T10:32:29.000-07:00
diff --git a/macos/MenuBarApp/FileIndexerWindow.swift b/macos/MenuBarApp/FileIndexerWindow.swift
@@ -441,16 +441,23 @@ class FileWatchManager: ObservableObject {
         // Stop watching
         stopWatching(folder.path)
         
+        // Delete all nodes from NornicDB in background
+        Task {
+            do {
+                try await deleteNodesForFolder(folder.path)
+            } catch {
+                print("⚠️  Failed to delete nodes for folder \(folder.path): \(error.localizedDescription)")
+                // Continue with removal even if DB cleanup fails
+            }
+        }
+        
         // Remove from list
         DispatchQueue.main.async {
             self.watchedFolders.removeAll { $0.id == folder.id }
             self.progressMap.removeValue(forKey: folder.path)
             self.saveFolders()
             self.updateStats()
         }
-        
-        // TODO: Remove nodes from NornicDB
-        // await deleteNodesForFolder(folder.path)
     }
     
     func refreshFolder(_ folder: IndexedFolder) {
@@ -764,21 +771,62 @@ class FileWatchManager: ObservableObject {
         }
     }
     
-    /// Delete all file nodes for a folder
+    /// Delete all file nodes and their chunks for a folder
     func deleteNodesForFolder(_ folderPath: String) async throws {
-        let query = """
+        // First, get count of nodes to be deleted for logging
+        let countQuery = """
+        MATCH (f:File {folder_root: $folder_root})
+        OPTIONAL MATCH (f)-[:HAS_CHUNK]->(c)
+        RETURN count(DISTINCT f) as fileCount, count(DISTINCT c) as chunkCount
+        """
+        
+        let countBody: [String: Any] = [
+            "statements": [
+                ["statement": countQuery, "parameters": ["folder_root": folderPath]]
+            ]
+        ]
+        
+        let countData = try JSONSerialization.data(withJSONObject: countBody)
+        let (countResponseData, _) = try await makeAuthenticatedRequest(to: "/db/neo4j/tx/commit", method: "POST", body: countData)
+        
+        var fileCount = 0
+        var chunkCount = 0
+        if let json = try? JSONSerialization.jsonObject(with: countResponseData) as? [String: Any],
+           let results = json["results"] as? [[String: Any]],
+           let data = results.first?["data"] as? [[String: Any]],
+           let row = data.first?["row"] as? [Int] {
+            fileCount = row[0]
+            chunkCount = row[1]
+        }
+        
+        print("🗑️  Deleting \(fileCount) File nodes and \(chunkCount) FileChunk nodes for folder: \(folderPath)")
+        
+        // Delete all File nodes and their associated FileChunk nodes
+        // DETACH DELETE on File removes HAS_CHUNK relationships
+        // Then explicitly delete orphaned FileChunk nodes
+        let deleteQuery = """
         MATCH (f:File {folder_root: $folder_root})
+        OPTIONAL MATCH (f)-[:HAS_CHUNK]->(c)
+        WITH f, collect(c) as chunks
         DETACH DELETE f
+        FOREACH (chunk IN chunks | DETACH DELETE chunk)
         """
         
-        let body: [String: Any] = [
+        let deleteBody: [String: Any] = [
             "statements": [
-                ["statement": query, "parameters": ["folder_root": folderPath]]
+                ["statement": deleteQuery, "parameters": ["folder_root": folderPath]]
             ]
         ]
         
-        let bodyData = try JSONSerialization.data(withJSONObject: body)
-        let _ = try await makeAuthenticatedRequest(to: "/db/neo4j/tx/commit", method: "POST", body: bodyData)
+        let deleteData = try JSONSerialization.data(withJSONObject: deleteBody)
+        let (_, response) = try await makeAuthenticatedRequest(to: "/db/neo4j/tx/commit", method: "POST", body: deleteData)
+        
+        if response.statusCode == 200 {
+            print("✅ Successfully deleted all nodes for folder: \(folderPath)")
+        } else {
+            throw NSError(domain: "FileWatchManager", code: response.statusCode,
+                         userInfo: [NSLocalizedDescriptionKey: "Failed to delete nodes from NornicDB"])
+        }
     }
     
     /// Perform vector search
diff --git a/pkg/cypher/executor.go b/pkg/cypher/executor.go
@@ -1395,21 +1395,24 @@ func (e *StorageExecutor) executeDelete(ctx context.Context, cypher string) (*Ex
 			}
 
 			if detach {
-				// Delete all connected edges first
-				edges, _ := e.storage.GetOutgoingEdges(storage.NodeID(nodeID))
-				for _, edge := range edges {
-					e.storage.DeleteEdge(edge.ID)
-					result.Stats.RelationshipsDeleted++
+				// Count edges that will be deleted with the node (for stats)
+				// DeleteNode() automatically deletes connected edges and updates counts internally
+				// We just need to count them for the result stats
+				outgoingEdges, _ := e.storage.GetOutgoingEdges(storage.NodeID(nodeID))
+				incomingEdges, _ := e.storage.GetIncomingEdges(storage.NodeID(nodeID))
+				edgesCount := len(outgoingEdges) + len(incomingEdges)
+
+				// DeleteNode() handles edge deletion internally and updates internal counts
+				if err := e.storage.DeleteNode(storage.NodeID(nodeID)); err == nil {
+					result.Stats.NodesDeleted++
+					result.Stats.RelationshipsDeleted += edgesCount
 				}
-				edges, _ = e.storage.GetIncomingEdges(storage.NodeID(nodeID))
-				for _, edge := range edges {
-					e.storage.DeleteEdge(edge.ID)
-					result.Stats.RelationshipsDeleted++
+			} else {
+				// Non-detach delete - just delete the node (will fail if edges exist)
+				if err := e.storage.DeleteNode(storage.NodeID(nodeID)); err == nil {
+					result.Stats.NodesDeleted++
 				}
 			}
-			if err := e.storage.DeleteNode(storage.NodeID(nodeID)); err == nil {
-				result.Stats.NodesDeleted++
-			}
 		}
 	}
 
diff --git a/pkg/storage/async_engine.go b/pkg/storage/async_engine.go
@@ -14,6 +14,7 @@ package storage
 import (
 	"context"
 	"fmt"
+	"log"
 	"strings"
 	"sync"
 	"time"
@@ -384,6 +385,11 @@ func (ae *AsyncEngine) DeleteNode(id NodeID) error {
 	ae.mu.Lock()
 	defer ae.mu.Unlock()
 
+	// Check if already marked for deletion (idempotent)
+	if ae.deleteNodes[id] {
+		return nil
+	}
+
 	// Check if node is being flushed right now (in-flight)
 	isInFlight := ae.inFlightNodes[id]
 
@@ -408,7 +414,21 @@ func (ae *AsyncEngine) DeleteNode(id NodeID) error {
 		return nil
 	}
 
-	// Node exists in underlying engine (or will after in-flight flush) - mark for deletion
+	// If in-flight, it will exist in underlying engine after flush - mark for deletion
+	if isInFlight {
+		ae.deleteNodes[id] = true
+		ae.pendingWrites++
+		return nil
+	}
+
+	// Check if node actually exists in underlying engine before marking for deletion
+	// This prevents count going negative for non-existent nodes
+	if _, err := ae.engine.GetNode(id); err != nil {
+		// Node doesn't exist anywhere - nothing to delete
+		return ErrNotFound
+	}
+
+	// Node exists in underlying engine - mark for deletion
 	ae.deleteNodes[id] = true
 	ae.pendingWrites++
 	return nil
@@ -444,6 +464,11 @@ func (ae *AsyncEngine) DeleteEdge(id EdgeID) error {
 	ae.mu.Lock()
 	defer ae.mu.Unlock()
 
+	// Check if already marked for deletion (idempotent)
+	if ae.deleteEdges[id] {
+		return nil
+	}
+
 	// Check if edge is being flushed right now (in-flight)
 	isInFlight := ae.inFlightEdges[id]
 
@@ -461,7 +486,21 @@ func (ae *AsyncEngine) DeleteEdge(id EdgeID) error {
 		return nil
 	}
 
-	// Edge exists in underlying engine (or will after in-flight flush) - mark for deletion
+	// If in-flight, it will exist in underlying engine after flush - mark for deletion
+	if isInFlight {
+		ae.deleteEdges[id] = true
+		ae.pendingWrites++
+		return nil
+	}
+
+	// Check if edge actually exists in underlying engine before marking for deletion
+	// This prevents count going negative for non-existent edges
+	if _, err := ae.engine.GetEdge(id); err != nil {
+		// Edge doesn't exist anywhere - nothing to delete
+		return ErrNotFound
+	}
+
+	// Edge exists in underlying engine - mark for deletion
 	ae.deleteEdges[id] = true
 	ae.pendingWrites++
 	return nil
@@ -886,16 +925,22 @@ func (ae *AsyncEngine) NodeCount() (int64, error) {
 	}
 	pendingDeletes := int64(len(ae.deleteNodes))
 
-	count, err := ae.engine.NodeCount()
+	engineCount, err := ae.engine.NodeCount()
 	ae.mu.RUnlock()
 
 	if err != nil {
 		return 0, err
 	}
 
 	// Adjust for pending creates and deletes
-	count += pendingCreates
-	count -= pendingDeletes
+	count := engineCount + pendingCreates - pendingDeletes
+
+	// Clamp to zero if negative (should never happen, log for debugging)
+	if count < 0 {
+		log.Printf("⚠️ [COUNT BUG] NodeCount went negative: engineCount=%d pendingCreates=%d pendingDeletes=%d result=%d (clamping to 0)",
+			engineCount, pendingCreates, pendingDeletes, count)
+		return 0, nil
+	}
 	return count, nil
 }
 
@@ -915,16 +960,22 @@ func (ae *AsyncEngine) EdgeCount() (int64, error) {
 	}
 	pendingDeletes := int64(len(ae.deleteEdges))
 
-	count, err := ae.engine.EdgeCount()
+	engineCount, err := ae.engine.EdgeCount()
 	ae.mu.RUnlock()
 
 	if err != nil {
 		return 0, err
 	}
 
 	// Adjust for pending creates and deletes
-	count += pendingCreates
-	count -= pendingDeletes
+	count := engineCount + pendingCreates - pendingDeletes
+
+	// Clamp to zero if negative (should never happen, log for debugging)
+	if count < 0 {
+		log.Printf("⚠️ [COUNT BUG] EdgeCount went negative: engineCount=%d pendingCreates=%d pendingDeletes=%d result=%d (clamping to 0)",
+			engineCount, pendingCreates, pendingDeletes, count)
+		return 0, nil
+	}
 	return count, nil
 }
 
diff --git a/pkg/storage/async_engine_delete_stats_test.go b/pkg/storage/async_engine_delete_stats_test.go
@@ -206,6 +206,69 @@ func TestDetachDeleteStatsTracking(t *testing.T) {
 		assert.Equal(t, int64(1), nc, "After flush: 1 node (nodeB)")
 		assert.Equal(t, int64(0), ec, "After flush: 0 edges")
 	})
+
+	t.Run("delete_nonexistent_node_does_not_affect_count", func(t *testing.T) {
+		// This tests the critical fix: deleting a non-existent node should NOT
+		// decrement the count and cause it to go negative
+		engine := NewMemoryEngine()
+		asyncConfig := &AsyncEngineConfig{
+			FlushInterval: 100 * time.Millisecond,
+		}
+		asyncEngine := NewAsyncEngine(engine, asyncConfig)
+		defer asyncEngine.Close()
+
+		// Initial count should be 0
+		count, err := asyncEngine.NodeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Initial count should be 0")
+
+		// Try to delete a non-existent node
+		err = asyncEngine.DeleteNode("nonexistent_node_id")
+		assert.Equal(t, ErrNotFound, err, "Deleting non-existent node should return ErrNotFound")
+
+		// Count should still be 0, not -1
+		count, err = asyncEngine.NodeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Count should remain 0 after deleting non-existent node")
+
+		// Flush and verify
+		require.NoError(t, asyncEngine.Flush())
+
+		count, err = asyncEngine.NodeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Count should remain 0 after flush")
+	})
+
+	t.Run("delete_nonexistent_edge_does_not_affect_count", func(t *testing.T) {
+		// Similar test for edges
+		engine := NewMemoryEngine()
+		asyncConfig := &AsyncEngineConfig{
+			FlushInterval: 100 * time.Millisecond,
+		}
+		asyncEngine := NewAsyncEngine(engine, asyncConfig)
+		defer asyncEngine.Close()
+
+		// Initial count should be 0
+		count, err := asyncEngine.EdgeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Initial edge count should be 0")
+
+		// Try to delete a non-existent edge
+		err = asyncEngine.DeleteEdge("nonexistent_edge_id")
+		assert.Equal(t, ErrNotFound, err, "Deleting non-existent edge should return ErrNotFound")
+
+		// Count should still be 0
+		count, err = asyncEngine.EdgeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Edge count should remain 0 after deleting non-existent edge")
+
+		// Flush and verify
+		require.NoError(t, asyncEngine.Flush())
+
+		count, err = asyncEngine.EdgeCount()
+		require.NoError(t, err)
+		assert.Equal(t, int64(0), count, "Edge count should remain 0 after flush")
+	})
 }
 
 // TestBadgerEngineDetachDeleteStats tests the underlying BadgerEngine directly
diff --git a/pkg/storage/badger.go b/pkg/storage/badger.go
@@ -2286,7 +2286,14 @@ func (b *BadgerEngine) NodeCount() (int64, error) {
 
 	// Return cached count for O(1) performance
 	// The counter is updated atomically on create/delete operations
-	return b.nodeCount.Load(), nil
+	count := b.nodeCount.Load()
+
+	// Clamp to zero if negative (should never happen, log for debugging)
+	if count < 0 {
+		log.Printf("⚠️ [COUNT BUG] BadgerEngine.NodeCount went negative: %d (clamping to 0)", count)
+		return 0, nil
+	}
+	return count, nil
 }
 
 // EdgeCount returns the total number of valid, decodable edges.
@@ -2301,7 +2308,14 @@ func (b *BadgerEngine) EdgeCount() (int64, error) {
 
 	// Return cached count for O(1) performance
 	// The counter is updated atomically on create/delete operations
-	return b.edgeCount.Load(), nil
+	count := b.edgeCount.Load()
+
+	// Clamp to zero if negative (should never happen, log for debugging)
+	if count < 0 {
+		log.Printf("⚠️ [COUNT BUG] BadgerEngine.EdgeCount went negative: %d (clamping to 0)", count)
+		return 0, nil
+	}
+	return count, nil
 }
 
 // GetSchema returns the schema manager.
