Address code review feedback for IncludeGraph feature

Co-authored-by: PhenX <42170+PhenX@users.noreply.github.com>

Author: Copilot

docs/graph-insert.md

@@ -1,6 +1,6 @@
 # Graph Insert (Navigation Properties)
 
-> ℹ️ This feature is not available for Oracle and MySQL providers due to limitations in retrieving generated IDs.
+> ℹ️ Graph inserts that require database-generated key propagation are not supported for Oracle and MySQL providers due to limitations in retrieving generated IDs. Graph inserts using client-generated keys (e.g., GUIDs with `ValueGeneratedNever()`) are supported on all providers.
 
 This library supports bulk inserting entire object graphs, including entities with their related navigation properties.
 
@@ -19,7 +19,7 @@ await dbContext.ExecuteBulkInsertAsync(blogs, options =>
 2. Entities are sorted in topological order (parents before children) to respect foreign key constraints
 3. Each entity type is bulk inserted in dependency order
 4. Generated IDs (identity columns) are propagated to foreign key properties
-5. Many-to-many join tables are populated automatically
+5. Many-to-many join tables with explicit join entity types are populated automatically (see Limitations below)
 
 ## Options
 

src/PhenX.EntityFrameworkCore.BulkInsert/Graph/GraphBulkInsertOrchestrator.cs

@@ -177,11 +177,12 @@ private async Task InsertEntitiesOfType(
             .GetMethod(nameof(InsertEntitiesGenericAsync), BindingFlags.NonPublic | BindingFlags.Instance)!
             .MakeGenericMethod(entityType);
 
-        var task = (Task)method.Invoke(this, [context, entities, options, provider, graphMetadata, ctk])!;
+        var task = (Task)method.Invoke(this, [sync, context, entities, options, provider, graphMetadata, ctk])!;
         await task;
     }
 
     private async Task InsertEntitiesGenericAsync<TEntity>(
+        bool sync,
         DbContext context,
         List<object> entities,
         BulkInsertOptions options,
@@ -200,7 +201,7 @@ private async Task InsertEntitiesGenericAsync<TEntity>(
             // Use BulkInsertReturnEntities to get back the generated IDs
             var insertedEntities = new List<TEntity>();
             await foreach (var inserted in provider.BulkInsertReturnEntities(
-                               false,
+                               sync,
                                context,
                                tableInfo,
                                typedEntities,
@@ -221,7 +222,7 @@ private async Task InsertEntitiesGenericAsync<TEntity>(
         else
         {
             // No identity columns, just insert directly
-            await provider.BulkInsert(false, context, tableInfo, typedEntities, options, null, ctk);
+            await provider.BulkInsert(sync, context, tableInfo, typedEntities, options, null, ctk);
         }
     }
 
@@ -380,6 +381,18 @@ private async Task InsertJoinEntities(
         IBulkInsertProvider provider,
         CancellationToken ctk)
     {
+        // Skip dictionary-based shared-type join entities as they are not supported
+        // by the bulk insert infrastructure (requires typed IEnumerable<T>)
+        if (joinEntityType == typeof(Dictionary<string, object>) ||
+            typeof(IDictionary<string, object>).IsAssignableFrom(joinEntityType))
+        {
+            _logger?.LogWarning(
+                "IncludeGraph: Skipping join table insertion for shared-type entity (Dictionary<string, object>). " +
+                "Many-to-many relationships using implicit join tables are not supported. " +
+                "Consider using an explicit join entity type.");
+            return;
+        }
+
         var efEntityType = context.Model.FindEntityType(joinEntityType);
         if (efEntityType == null)
         {
@@ -389,17 +402,26 @@ private async Task InsertJoinEntities(
         var sqlDialect = provider.SqlDialect;
         var tableInfo = new TableMetadata(efEntityType, sqlDialect);
 
-        // Use raw SQL insert for join entities since they're often dictionary-based
-        var method = typeof(IBulkInsertProvider)
-            .GetMethod(nameof(IBulkInsertProvider.BulkInsert))!
+        // Use reflection to call the generic BulkInsert method with correctly typed entities
+        var method = typeof(GraphBulkInsertOrchestrator)
+            .GetMethod(nameof(InsertJoinEntitiesGenericAsync), BindingFlags.NonPublic | BindingFlags.Instance)!
             .MakeGenericMethod(joinEntityType);
 
-        var result = method.Invoke(provider, [sync, context, tableInfo, joinEntities, options, null, ctk]);
-        if (result is not Task task)
-        {
-            throw new InvalidOperationException(
-                $"The BulkInsert method for join entity type '{joinEntityType.Name}' did not return a Task as expected.");
-        }
+        var task = (Task)method.Invoke(this, [sync, context, tableInfo, joinEntities, options, provider, ctk])!;
         await task;
     }
+
+    private async Task InsertJoinEntitiesGenericAsync<TJoin>(
+        bool sync,
+        DbContext context,
+        TableMetadata tableInfo,
+        List<object> joinEntities,
+        BulkInsertOptions options,
+        IBulkInsertProvider provider,
+        CancellationToken ctk) where TJoin : class
+    {
+        // Cast to correctly typed list for the provider
+        var typedEntities = joinEntities.Cast<TJoin>().ToList();
+        await provider.BulkInsert(sync, context, tableInfo, typedEntities, options, null, ctk);
+    }
 }

src/PhenX.EntityFrameworkCore.BulkInsert/Graph/GraphEntityCollector.cs

@@ -1,6 +1,7 @@
 using System.Collections;
 
 using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Metadata;
 
 using PhenX.EntityFrameworkCore.BulkInsert.Metadata;
 using PhenX.EntityFrameworkCore.BulkInsert.Options;
@@ -152,6 +153,15 @@ private static void SetInverseNavigation(object parentEntity, object childEntity
             return;
         }
 
+        // Only set inverse navigations that are reference properties (not collections).
+        // If the inverse is a collection, the parent should be added to the collection,
+        // not assigned directly (which would cause an invalid cast).
+        if (navigation.Navigation is INavigation { Inverse.IsCollection: true })
+        {
+            // Skip: inverse is a collection, not a reference property
+            return;
+        }
+
         // Check if the inverse navigation is already set
         var currentValue = navigation.GetInverseValue(childEntity);
         if (currentValue == null)

tests/PhenX.EntityFrameworkCore.BulkInsert.Tests/Tests/Graph/GraphTestsBase.cs

@@ -592,17 +592,17 @@ await _context.ExecuteBulkInsertAsync(blogs, options =>
     [SkippableFact]
     public async Task InsertGraph_LargeScale()
     {
-        // Arrange - Create many blogs with many children each (Posts, Tags, BlogSettings)
-        // This tests performance and correctness at scale
-        const int blogCount = 5000;
-        const int postsPerBlog = 100;
-        const int tagsPerPost = 10;
+        // Arrange - Create blogs with children each (Posts, Tags, BlogSettings)
+        // This tests correctness with a reasonable amount of data that won't cause CI timeouts
+        const int blogCount = 50;
+        const int postsPerBlog = 10;
+        const int tagsPerPost = 3;
 
         var blogs = new List<Blog>();
         var allTags = new List<Tag>();
 
         // Pre-create a pool of tags that will be shared across posts
-        for (var i = 0; i < 50; i++)
+        for (var i = 0; i < 20; i++)
         {
             allTags.Add(new Tag
             {