v0.16.0 performance optimisation and query enhancements

aryonoco · aryonoco · commit c3f774bae8cf · 2025-07-11T12:23:50.000+10:00
diff --git a/.gitignore b/.gitignore
@@ -98,6 +98,9 @@ llm-generated*
 concatenated_fsharp_files.txt
 concatenate_fsharp.sh
 
+# Debug Scripts
+debug-scripts/*
+
 
 
 # Distribution directory - contains built binaries
diff --git a/.version b/.version
@@ -1 +1 @@
-0.15.0
+0.16.0
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -1,7 +1,7 @@
 <Project>
   <PropertyGroup>
-    <Version>0.15.0</Version>
-    <AssemblyVersion>0.15.0.0</AssemblyVersion>
-    <FileVersion>0.15.0.0</FileVersion>
+    <Version>0.16.0</Version>
+    <AssemblyVersion>0.16.0.0</AssemblyVersion>
+    <FileVersion>0.16.0.0</FileVersion>
   </PropertyGroup>
 </Project>
diff --git a/Neo4jExport/src/Core/Types.fs b/Neo4jExport/src/Core/Types.fs
@@ -167,6 +167,7 @@ type AppError =
     | FileSystemError of path: string * message: string * exn: exn option
     | SecurityError of message: string
     | TimeoutError of operation: string * duration: TimeSpan
+    | PaginationError of entityType: string * message: string
     | AggregateError of NonEmptyList<AppError>
 
 /// Mutable context for managing application lifecycle and cleanup
diff --git a/Neo4jExport/src/ErrorManagement/ErrorAccumulation.fs b/Neo4jExport/src/ErrorManagement/ErrorAccumulation.fs
@@ -67,6 +67,7 @@ module ErrorAccumulation =
         | MemoryError msg -> msg
         | ExportError(msg, _) -> msg
         | TimeoutError(operation, duration) -> sprintf "Operation '%s' timed out after %A" operation duration
+        | PaginationError(entityType, msg) -> sprintf "Pagination error for %s: %s" entityType msg
         | AggregateError nel ->
             let errors = NonEmptyList.toList nel
 
diff --git a/Neo4jExport/src/Export/BatchProcessing.fs b/Neo4jExport/src/Export/BatchProcessing.fs
@@ -185,21 +185,8 @@ let processNodeRecord (exportState: ExportState) (recordCtx: RecordContext<Batch
         
         exportState.NodeIdMapping.TryAdd(elementId, stableId) |> ignore
         
-        writeNode writer { new INode with
-                            member _.ElementId = elementId
-                            member _.Labels = labels :> IReadOnlyList<string>
-                            member _.Properties = properties
-                            member _.Item with get(key: string) = properties.[key]
-                            member _.Get<'T>(key: string) : 'T = properties.[key] :?> 'T
-                            member _.TryGet<'T>(key: string, [<System.Runtime.InteropServices.Out>] value: byref<'T>) = 
-                                let success, v = properties.TryGetValue(key)
-                                if success && v :? 'T then
-                                    value <- unbox<'T> v
-                                    true
-                                else
-                                    false
-                            member _.Id = 0L  // Not used, but required by interface
-                            member _.Equals(other: INode) = false } elementId stableId ctx
+        // Use direct serialization to avoid object allocation in hot path
+        writeNodeDirect writer elementId stableId (labels :> IReadOnlyList<string>) properties ctx
     with ex ->
         let elementId = ""
 
@@ -310,25 +297,8 @@ let processRelationshipRecord
               EndElementId = endNodeId
               EndStableId = endStableId }  // End node content hash
 
-        writeRelationship writer { new IRelationship with
-                                     member _.ElementId = elementId
-                                     member _.Type = relType
-                                     member _.StartNodeElementId = startNodeId
-                                     member _.EndNodeElementId = endNodeId
-                                     member _.Properties = properties
-                                     member _.Item with get(key: string) = properties.[key]
-                                     member _.Get<'T>(key: string) : 'T = properties.[key] :?> 'T
-                                     member _.TryGet<'T>(key: string, [<System.Runtime.InteropServices.Out>] value: byref<'T>) = 
-                                         let success, v = properties.TryGetValue(key)
-                                         if success && v :? 'T then
-                                             value <- unbox<'T> v
-                                             true
-                                         else
-                                             false
-                                     member _.Id = 0L  // Not used, but required by interface
-                                     member _.StartNodeId = 0L  // Not used, but required by interface
-                                     member _.EndNodeId = 0L    // Not used, but required by interface
-                                     member _.Equals(other: IRelationship) = false } ids ctx
+        // Use direct serialization to avoid object allocation in hot path
+        writeRelationshipDirect writer relType properties ids ctx
     with ex ->
         // Failed to parse relationship from record
         trackSerializationErrorDedup recordCtx.ErrorAccumulator ex "" "relationship" "RecordAccessError"
@@ -383,19 +353,19 @@ module private KeysetPagination =
     /// Extract ID from record for pagination
     let extractId (version: Neo4jVersion) (entityType: string) (record: IRecord) : KeysetId option =
         try
-            let fieldName =
-                match entityType with
-                | "Nodes" -> "nodeId"
-                | "Relationships" -> "relId"
-                | _ -> failwithf "Unknown entity type: %s" entityType
-
             match version with
             | V4x ->
+                // Neo4j 4.x uses different field names for nodes and relationships
+                let fieldName =
+                    match entityType with
+                    | "Nodes" -> "nodeId"
+                    | "Relationships" -> "relId"
+                    | _ -> failwithf "Unknown entity type: %s" entityType
                 let id = record.[fieldName].As<int64>()
                 Some(NumericId id)
-            | V5x
-            | V6x ->
-                let id = record.[fieldName].As<string>()
+            | V5x | V6x ->
+                // Neo4j 5.x+ uses elementId for both nodes and relationships
+                let id = record.["elementId"].As<string>()
                 Some(ElementId id)
             | Unknown -> None
         with ex ->
@@ -629,7 +599,16 @@ let processBatchedQuery<'state>
 
                             // Continue if we got a full batch
                             if recordCount = batchSize then
-                                return! processBatch batchStats newLastProgress nextPaginationState currentHandlerState
+                                // Critical check: Prevent infinite loop when using keyset pagination
+                                match paginationState, nextPaginationState with
+                                | Keyset(prevId, _), Keyset(nextId, _) when prevId = nextId ->
+                                    // We processed a full batch but couldn't advance the pagination cursor
+                                    // This means ID extraction failed for all records in the batch
+                                    return Error(PaginationError(
+                                        batchCtx.Processor.EntityName,
+                                        sprintf "Unable to advance pagination after processing %d records. This typically occurs when the ID field (nodeId/relId) cannot be extracted from query results. Check that your Neo4j query returns the expected fields." recordCount))
+                                | _ ->
+                                    return! processBatch batchStats newLastProgress nextPaginationState currentHandlerState
                             else
                                 return Ok(batchStats, currentHandlerState)
             }
diff --git a/Neo4jExport/src/Export/Core.fs b/Neo4jExport/src/Export/Core.fs
@@ -43,7 +43,7 @@ module QueryBuilders =
         | V5x
         | V6x
         | Unknown ->
-            "MATCH (n) RETURN n, labels(n) as labels, elementId(n) as nodeId ORDER BY elementId(n) SKIP $skip LIMIT $limit"
+            "MATCH (n) RETURN n, labels(n) as labels, elementId(n) as elementId ORDER BY elementId(n) SKIP $skip LIMIT $limit"
 
     /// Build node query for future keyset pagination
     let buildNodeQueryKeyset (version: Neo4jVersion) (lastId: KeysetId option) =
@@ -80,8 +80,7 @@ module QueryBuilders =
             RETURN 
                 elementId(n) AS elementId,
                 labels(n) AS labels,
-                n AS node,
-                elementId(n) AS nodeId
+                n AS node
             ORDER BY elementId(n)
             LIMIT $limit
             """ whereClause
@@ -93,7 +92,7 @@ module QueryBuilders =
         | V5x
         | V6x
         | Unknown ->
-            "MATCH ()-[r]->() RETURN r, type(r) as type, elementId(r) as relId ORDER BY elementId(r) SKIP $skip LIMIT $limit"
+            "MATCH ()-[r]->() RETURN r, type(r) as type, elementId(r) as elementId ORDER BY elementId(r) SKIP $skip LIMIT $limit"
 
     /// Build relationship query for future keyset pagination
     let buildRelationshipQueryKeyset (version: Neo4jVersion) (lastId: KeysetId option) =
@@ -134,8 +133,7 @@ module QueryBuilders =
                 type(r) AS type,
                 elementId(startNode) AS startNodeElementId,
                 elementId(endNode) AS endNodeElementId,
-                r AS relationship,
-                elementId(r) AS relId
+                r AS relationship
             ORDER BY elementId(r)
             LIMIT $limit
             """ whereClause
@@ -292,20 +290,23 @@ let exportNodesUnified
                 with _ ->
                     []
 
-            let bytesPerLabel =
-                if List.isEmpty labels then
-                    bytesWritten
-                else
-                    bytesWritten / int64 labels.Length
-
+            // Handle both labeled and unlabeled nodes
             let newLabelTracker =
-                labels
-                |> List.fold
-                    (fun tracker label ->
-                        tracker
-                        |> LabelStatsTracker.startLabel label
-                        |> LabelStatsTracker.updateLabel label 1L bytesPerLabel)
+                if List.isEmpty labels then
+                    // Track unlabeled nodes under a special category
                     state.LabelTracker
+                    |> LabelStatsTracker.startLabel "_unlabeled"
+                    |> LabelStatsTracker.updateLabel "_unlabeled" 1L bytesWritten
+                else
+                    // Distribute bytes evenly among labels
+                    let bytesPerLabel = bytesWritten / int64 labels.Length
+                    labels
+                    |> List.fold
+                        (fun tracker label ->
+                            tracker
+                            |> LabelStatsTracker.startLabel label
+                            |> LabelStatsTracker.updateLabel label 1L bytesPerLabel)
+                        state.LabelTracker
 
             { LineState = newLineState
               LabelTracker = newLabelTracker }
diff --git a/Neo4jExport/src/Export/Serialization/Engine.fs b/Neo4jExport/src/Export/Serialization/Engine.fs
@@ -75,6 +75,13 @@ let writeNode =
 let writeRelationship =
     SerializationGraphElements.writeRelationship
 
+/// Re-export high-performance direct functions for hot path
+let internal writeNodeDirect =
+    SerializationGraphElements.writeNodeDirect
+
+let internal writeRelationshipDirect =
+    SerializationGraphElements.writeRelationshipDirect
+
 let serializeProperties =
     SerializationCollections.serializeProperties
 
diff --git a/Neo4jExport/src/Export/Serialization/GraphElements.fs b/Neo4jExport/src/Export/Serialization/GraphElements.fs
@@ -23,6 +23,7 @@
 module Neo4jExport.SerializationGraphElements
 
 open System
+open System.Collections.Generic
 open System.Text.Json
 open Neo4j.Driver
 open Neo4jExport
@@ -122,15 +123,23 @@ let serializeRelationship
         writer.WriteEndObject()
         writer.WriteEndObject()
 
-let writeNode (writer: Utf8JsonWriter) (node: INode) (elementId: string) (stableId: string) (ctx: WriterContext) =
+/// Internal high-performance node serialization that works directly with primitive data
+/// Used in the hot path to avoid object allocations
+let internal writeNodeDirect 
+    (writer: Utf8JsonWriter) 
+    (elementId: string) 
+    (stableId: string) 
+    (labels: IReadOnlyList<string>) 
+    (properties: IReadOnlyDictionary<string, obj>) 
+    (ctx: WriterContext) =
     writer.WriteStartObject()
     writer.WriteString("type", "node")
     writer.WriteString("element_id", elementId)
     writer.WriteString("NET_node_content_hash", stableId)
     writer.WriteString("export_id", ctx.ExportId.ToString())
     writer.WriteStartArray("labels")
 
-    node.Labels
+    labels
     |> Seq.truncate ctx.Config.MaxLabelsPerNode
     |> Seq.iter (fun label ->
         match validateLabel label elementId with
@@ -142,20 +151,31 @@ let writeNode (writer: Utf8JsonWriter) (node: INode) (elementId: string) (stable
     writer.WriteEndArray()
     writer.WriteStartObject "properties"
 
-    serializeProperties writer ctx SerializationDepth.zero node.Properties
+    serializeProperties writer ctx SerializationDepth.zero properties
 
     writer.WriteEndObject()
     writer.WriteEndObject()
 
-let writeRelationship (writer: Utf8JsonWriter) (rel: IRelationship) (ids: EntityIdsWithStable) (ctx: WriterContext) =
+let writeNode (writer: Utf8JsonWriter) (node: INode) (elementId: string) (stableId: string) (ctx: WriterContext) =
+    // Delegate to the direct implementation for code reuse
+    writeNodeDirect writer elementId stableId node.Labels node.Properties ctx
+
+/// Internal high-performance relationship serialization that works directly with primitive data
+/// Used in the hot path to avoid object allocations
+let internal writeRelationshipDirect
+    (writer: Utf8JsonWriter)
+    (relType: string)
+    (properties: IReadOnlyDictionary<string, obj>)
+    (ids: EntityIdsWithStable)
+    (ctx: WriterContext) =
     writer.WriteStartObject()
     writer.WriteString("type", "relationship")
     writer.WriteString("element_id", ids.ElementId)
     writer.WriteString("NET_rel_identity_hash", ids.StableId)
     writer.WriteString("export_id", ctx.ExportId.ToString())
 
     let safeType =
-        match validateRelType rel.Type ids.ElementId with
+        match validateRelType relType ids.ElementId with
         | Ok t -> t
         | Error msg ->
             ctx.ErrorFuncs.TrackWarning msg (Some ids.ElementId) None
@@ -168,11 +188,15 @@ let writeRelationship (writer: Utf8JsonWriter) (rel: IRelationship) (ids: Entity
     writer.WriteString("end_node_content_hash", ids.EndStableId)
     writer.WriteStartObject("properties")
 
-    serializeProperties writer ctx SerializationDepth.zero rel.Properties
+    serializeProperties writer ctx SerializationDepth.zero properties
 
     writer.WriteEndObject()
     writer.WriteEndObject()
 
+let writeRelationship (writer: Utf8JsonWriter) (rel: IRelationship) (ids: EntityIdsWithStable) (ctx: WriterContext) =
+    // Delegate to the direct implementation for code reuse
+    writeRelationshipDirect writer rel.Type rel.Properties ids ctx
+
 let serializeGraphElement
     (writer: Utf8JsonWriter)
     (ctx: WriterContext)
diff --git a/Neo4jExport/src/Export/Serialization/Path.fs b/Neo4jExport/src/Export/Serialization/Path.fs
@@ -32,24 +32,38 @@ open Neo4jExport.SerializationCollections
 open Neo4jExport.JsonHelpers
 open ErrorTracking
 
-let private generatePathSequenceTailRec nodeCount relCount =
-    let rec loop acc nodeIdx relIdx isNode =
-        match nodeIdx < nodeCount, relIdx < relCount, isNode with
-        | true, _, true ->
-            let item =
-                {| Type = "node"; Index = nodeIdx |}
-
-            loop (item :: acc) (nodeIdx + 1) relIdx false
-        | _, true, false ->
-            let item =
-                {| Type = "relationship"
-                   Index = relIdx |}
-
-            loop (item :: acc) nodeIdx (relIdx + 1) true
-        | false, false, _ -> List.rev acc
-        | _ -> loop acc nodeIdx relIdx (not isNode)
-
-    loop [] 0 0 true
+/// Generate the alternating sequence of nodes and relationships for a path
+/// Neo4j paths always follow the pattern: Node -> Relationship -> Node -> ... -> Node
+/// Therefore: relationships = nodes - 1 for valid paths
+let private generatePathSequence nodeCount relCount =
+    // Validate Neo4j path invariant
+    if relCount <> nodeCount - 1 && nodeCount > 0 then
+        // Log warning but continue - defensive against malformed data
+        Log.warn (sprintf "Invalid path structure: %d nodes and %d relationships (expected %d relationships)" 
+                    nodeCount relCount (nodeCount - 1))
+    
+    // Simple approach: generate indices for the alternating pattern
+    // Total elements = nodes + relationships
+    let totalElements = nodeCount + relCount
+    
+    // Handle edge cases
+    if totalElements = 0 then
+        []
+    else
+        [0 .. totalElements - 1]
+        |> List.map (fun i ->
+            if i % 2 = 0 then
+                // Even positions are nodes: 0, 2, 4, ...
+                {| Type = "node"; Index = i / 2 |}
+            else
+                // Odd positions are relationships: 1, 3, 5, ...
+                {| Type = "relationship"; Index = i / 2 |})
+        |> List.filter (fun item ->
+            // Defensive: ensure we don't exceed actual counts
+            match item.Type with
+            | "node" -> item.Index < nodeCount
+            | "relationship" -> item.Index < relCount
+            | _ -> false)
 
 let private serializePathFull (writer: Utf8JsonWriter) (ctx: WriterContext) (path: IPath) =
     writer.WritePropertyName("nodes")
@@ -179,7 +193,7 @@ let serializePath (writer: Utf8JsonWriter) (ctx: WriterContext) (path: IPath) =
         writer.WritePropertyName("sequence")
         writer.WriteStartArray()
 
-        generatePathSequenceTailRec path.Nodes.Count path.Relationships.Count
+        generatePathSequence path.Nodes.Count path.Relationships.Count
         |> List.iter (fun item ->
             writer.WriteStartObject()
             writer.WriteString("type", item.Type)
diff --git a/Neo4jExport/src/Workflow/Workflow.fs b/Neo4jExport/src/Workflow/Workflow.fs
@@ -96,6 +96,7 @@ module Workflow =
         | FileSystemError _ -> 3
         | SecurityError _ -> 6
         | TimeoutError _ -> 5
+        | PaginationError _ -> 7  // Same as QueryError - data access issue
         | AggregateError _ -> 6
 
     /// Prepare export file with metadata placeholder
