wip pipe implementation

dev -> main

.github/workflows/pr.yml

@@ -20,7 +20,7 @@ jobs:
           dotnet-version: 8.0.4xx # Align with global.json (including roll forward rules)
 
       - name: Cache Nuget
-        uses: actions/cache@v4
+        uses: actions/cache@v5
         with:
           path: ~/.nuget/packages
           key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}

.github/workflows/release.yml

@@ -26,7 +26,7 @@ jobs:
           dotnet-version: 8.0.4xx # Align with global.json (including roll forward rules)
 
       - name: Cache Nuget
-        uses: actions/cache@v4
+        uses: actions/cache@v5
         with:
           path: ~/.nuget/packages
           key: ${{ runner.os }}-nuget-${{ hashFiles('**/packages.lock.json') }}
@@ -35,7 +35,7 @@ jobs:
         run: ./build.ps1 zip
 
       - name: ⬆️ Upload artifacts
-        uses: actions/upload-artifact@v5
+        uses: actions/upload-artifact@v6
         with:
           name: output-${{ env.SEMVER }}
           path: output/*.*

Connectors/Revit/Speckle.Connectors.RevitShared/Operations/Send/RevitRootObjectBuilder.cs

@@ -42,7 +42,11 @@ public class RevitRootObjectBuilder(
       () => Task.FromResult(BuildSync(documentElementContexts, projectId, onOperationProgressed, ct))
     );
 
+#pragma warning disable CA1506
+#pragma warning disable CA1502
   private RootObjectBuilderResult BuildSync(
+#pragma warning restore CA1506
+#pragma warning restore CA1502
     IReadOnlyList<DocumentToConvert> documentElementContexts,
     string projectId,
     IProgress<CardProgress> onOperationProgressed,
@@ -56,6 +60,9 @@ public class RevitRootObjectBuilder(
       throw new SpeckleException("Family Environment documents are not supported.");
     }
 
+    // create a new send pipeline
+    using var sendPipeline = new Speckle.Sdk.Pipeline.Send();
+
     // init the root
     Collection rootObject =
       new() { name = converterSettings.Current.Document.PathName.Split('\\').Last().Split('.').First() };
@@ -184,10 +191,12 @@ public class RevitRootObjectBuilder(
             // non-transformed elements can safely rely on cache
             // TODO: Potential here to transform cached objects and NOT reconvert,
             // TODO: we wont do !hasTransform here, and re-set application id before this
-
+            bool wasCached = false;
             if (!hasTransform && sendConversionCache.TryGetValue(projectId, applicationId, out ObjectReference? value))
             {
+              // TODO: cahce hit
               converted = value;
+              wasCached = true;
               cacheHitCount++;
             }
             // not in cache means we convert
@@ -206,6 +215,12 @@ public class RevitRootObjectBuilder(
               converted.applicationId = applicationId;
             }
 
+            var reference = sendPipeline.Process(converted).Result; // .Wait(cancellationToken);//.ConfigureAwait(false);
+            if (!wasCached)
+            {
+              sendConversionCache.AppendSendResult(projectId, applicationId, reference);
+            }
+
             var collection = sendCollectionManager.GetAndCreateObjectHostCollection(
               revitElement,
               rootObject,
@@ -213,7 +228,7 @@ public class RevitRootObjectBuilder(
               modelDisplayName
             );
 
-            collection.elements.Add(converted);
+            collection.elements.Add(reference);
             results.Add(new(Status.SUCCESS, applicationId, sourceType, converted));
           }
           catch (Exception ex) when (!ex.IsFatal())
@@ -254,13 +269,20 @@ public class RevitRootObjectBuilder(
     rootObject[ProxyKeys.INSTANCE_DEFINITION] = revitToSpeckleCacheSingleton.GetInstanceDefinitionProxiesForObjects(
       idsAndSubElementIds
     );
-    rootObject.elements.Add(
-      new Collection()
-      {
-        elements = revitToSpeckleCacheSingleton.GetBaseObjectsForObjects(idsAndSubElementIds),
-        name = "revitInstancedObjects"
-      }
-    );
+    // NOTE: i might be overdoing things in here, but tldr:
+    // - all instance objects (meshes) are processed individually
+    // - process their collection individually, and then attach it to the root collection
+    // we could, theoretically, just process the collection as a whole (but it can be big?)
+    // note/ask: do these need to go in the conversion cache? or not?
+    var instanceObjects = revitToSpeckleCacheSingleton.GetBaseObjectsForObjects(idsAndSubElementIds);
+    var instanceReferences = new Collection("revitInstancedObjects");
+    foreach (var instanceObject in instanceObjects)
+    {
+      var referenceInstanceObject = sendPipeline.Process(instanceObject).Result;
+      instanceReferences.elements.Add(referenceInstanceObject);
+    }
+    var instanceReferenceCollection = sendPipeline.Process(instanceReferences).Result;
+    rootObject.elements.Add(instanceReferenceCollection);
 
     // STEP 6: Unpack all other objects to attach to root collection
     List<Objects.Other.Camera> views = viewUnpacker.Unpack(converterSettings.Current.Document);
@@ -279,6 +301,10 @@ public class RevitRootObjectBuilder(
       rootObject[RootKeys.REFERENCE_POINT_TRANSFORM] = transformMatrix;
     }
 
-    return new RootObjectBuilderResult(rootObject, results);
+    // NOTE: could be
+    sendPipeline.Process(rootObject).Wait(cancellationToken);
+    sendPipeline.WaitForUpload().Wait(cancellationToken);
+
+    return new RootObjectBuilderResult(new Collection() { name = "ignore" }, results);
   }
 }

Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Send/RhinoRootObjectBuilder.cs

@@ -75,6 +75,8 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
   )
   {
     using var activity = _activityFactory.Start("Build");
+    using var sendPipeline = new Speckle.Sdk.Pipeline.Send();
+
     // 0 - Init the root
     Collection rootObjectCollection = new() { name = _converterSettings.Current.Document.Name ?? "Unnamed document" };
     rootObjectCollection["units"] = _converterSettings.Current.SpeckleUnits;
@@ -97,6 +99,7 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
     // 3 - Convert atomic objects
     List<SendConversionResult> results = new(atomicObjects.Count);
     int count = 0;
+
     using (var _ = _activityFactory.Start("Convert all"))
     {
       foreach (RhinoObject rhinoObject in atomicObjects)
@@ -108,9 +111,8 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
         Layer layer = _converterSettings.Current.Document.Layers[rhinoObject.Attributes.LayerIndex];
         Collection collectionHost = _layerUnpacker.GetHostObjectCollection(layer, rootObjectCollection);
 
-        var result = ConvertRhinoObject(rhinoObject, collectionHost, instanceProxies, projectId);
+        var result = await ConvertRhinoObject(rhinoObject, collectionHost, instanceProxies, projectId, sendPipeline);
         results.Add(result);
-
         ++count;
         onOperationProgressed.Report(new("Converting", (double)count / atomicObjects.Count));
         await Task.Yield();
@@ -149,18 +151,23 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
       }
     }
 
-    return new RootObjectBuilderResult(rootObjectCollection, results);
+    await sendPipeline.Process(rootObjectCollection);
+    await sendPipeline.WaitForUpload();
+
+    return new RootObjectBuilderResult(new Collection() { name = "ignore" }, results);
   }
 
-  private SendConversionResult ConvertRhinoObject(
+  private async Task<SendConversionResult> ConvertRhinoObject(
     RhinoObject rhinoObject,
     Collection collectionHost,
     IReadOnlyDictionary<string, InstanceProxy> instanceProxies,
-    string projectId
+    string projectId,
+    Sdk.Pipeline.Send sendPipeline
   )
   {
     string applicationId = rhinoObject.Id.ToString();
     string sourceType = rhinoObject.ObjectType.ToString();
+    bool wasCached = false;
     try
     {
       // get from cache or convert:
@@ -174,6 +181,7 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
       else if (_sendConversionCache.TryGetValue(projectId, applicationId, out ObjectReference? value))
       {
         converted = value;
+        wasCached = true;
       }
       else
       {
@@ -194,10 +202,17 @@ public class RhinoRootObjectBuilder : IRootObjectBuilder<RhinoObject>
         converted["properties"] = properties;
       }
 
-      // add to host
-      collectionHost.elements.Add(converted);
+      // process in pipeline
+      var reference = await sendPipeline.Process(converted).ConfigureAwait(false);
+      if (!wasCached)
+      {
+        _sendConversionCache.AppendSendResult(projectId, applicationId, reference);
+      }
 
-      return new(Status.SUCCESS, applicationId, sourceType, converted);
+      // add to host
+      collectionHost.elements.Add(reference);
+
+      return new(Status.SUCCESS, applicationId, sourceType, reference);
     }
     catch (Exception ex) when (!ex.IsFatal())
     {

Connectors/Rhino/Speckle.Connectors.RhinoShared/Plugin/Speckle.Connectors.RhinoPlugin.cs

@@ -6,7 +6,7 @@ using Speckle.Connectors.Rhino.DependencyInjection;
 using Speckle.Converters.Rhino;
 using Speckle.Sdk;
 using Speckle.Sdk.Models.Extensions;
-
+  
 namespace Speckle.Connectors.Rhino.Plugin;
 
 ///<summary>

Converters/Revit/Speckle.Converters.RevitShared/Helpers/DisplayValueExtractor.cs

@@ -4,6 +4,7 @@ using Speckle.Converters.Common.ToSpeckle;
 using Speckle.Converters.RevitShared.Extensions;
 using Speckle.Converters.RevitShared.Services;
 using Speckle.Converters.RevitShared.Settings;
+using Speckle.Converters.RevitShared.ToSpeckle;
 using Speckle.DoubleNumerics;
 using Speckle.Objects;
 using Speckle.Sdk;
@@ -164,11 +165,11 @@ public sealed class DisplayValueExtractor
   }
 
   /// <summary>
-  /// Processes collections of different geometry types and converts them to display values.
-  /// Extracted as a common method to reduce code duplication between regular geometry processing and special cases like rebar.
+  /// Converts sorted geometry into DisplayValueResults <see cref="ElementTopLevelConverterToSpeckle"/>.
   /// </summary>
   /// <remarks>
-  /// Essentially all the ensuing steps after the common get_Geometry element method
+  /// Applies localToWorld only to curves, points, polylines.
+  /// Meshes remain in symbol space to generate correct instance proxies and avoid duplicates.
   /// </remarks>
   private List<DisplayValueResult> ProcessGeometryCollections(
     DB.Element element,
@@ -176,50 +177,35 @@ public sealed class DisplayValueExtractor
     DB.Transform? localToWorld
   )
   {
-    // handle all solids and meshes by their material
     var meshesByMaterial = GetMeshesByMaterial(collections.Meshes, collections.Solids);
-    List<SOG.Mesh> displayMeshes = _meshByMaterialConverter.Convert(
+    var displayMeshes = _meshByMaterialConverter.Convert(
       (meshesByMaterial, element.Id, ShouldSetElementDisplayToTransparent(element))
     );
 
     List<DisplayValueResult> displayValue = new(collections.TotalCount);
-    Matrix4x4? matrix = localToWorld is not null ? TransformToMatrix(localToWorld) : null;
 
-    foreach (SOG.Mesh mesh in displayMeshes)
+    foreach (var mesh in displayMeshes)
     {
+      // if we have a transform, keep mesh in symbol space and attach transform
       displayValue.Add(
-        matrix.HasValue
-          ? DisplayValueResult.WithTransform(mesh, matrix.Value)
+        localToWorld != null
+          ? DisplayValueResult.WithTransform(mesh, TransformToMatrix(localToWorld))
           : DisplayValueResult.WithoutTransform(mesh)
       );
     }
 
-    // transform curves, polylines, and points to world coordinates before conversion.
-    // Unlike meshes/solids which are proxified with transform matrices, these geometry
-    // types must have their final world coordinates baked directly into their geometry.
     foreach (var curve in collections.Curves)
     {
-      if (localToWorld is not null)
-      {
-        using var transformedCurve = curve.CreateTransformed(localToWorld);
-        displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(transformedCurve)));
-      }
-      else
-      {
-        displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(curve)));
-      }
+      var transformedCurve = localToWorld != null ? curve.CreateTransformed(localToWorld) : curve;
+      displayValue.Add(DisplayValueResult.WithoutTransform(GetCurveDisplayValue(transformedCurve)));
     }
 
-    // Note: Creating new polyline/point instances for transformation isn't ideal for perf,
-    // but Revit API doesn't provide in-place transform methods. Trade-off is acceptable since
-    // family instances typically don't have massive numbers of raw polylines/points in their geometry.
     foreach (var polyline in collections.Polylines)
     {
-      if (localToWorld is not null)
+      if (localToWorld != null)
       {
-        var coords = polyline.GetCoordinates();
-        var transformedCoords = coords.Select(coord => localToWorld.OfPoint(coord)).ToList();
-        using var transformedPolyline = DB.PolyLine.Create(transformedCoords);
+        var coords = polyline.GetCoordinates().Select(p => localToWorld.OfPoint(p)).ToList();
+        using var transformedPolyline = DB.PolyLine.Create(coords);
         displayValue.Add(DisplayValueResult.WithoutTransform(_polylineConverter.Convert(transformedPolyline)));
       }
       else
@@ -230,7 +216,7 @@ public sealed class DisplayValueExtractor
 
     foreach (var point in collections.Points)
     {
-      if (localToWorld is not null)
+      if (localToWorld != null)
       {
         using var transformedPoint = DB.Point.Create(localToWorld.OfPoint(point.Coord));
         displayValue.Add(DisplayValueResult.WithoutTransform(_pointConverter.Convert(transformedPoint)));
@@ -330,23 +316,17 @@ public sealed class DisplayValueExtractor
     };
 
   /// <summary>
-  /// According to the remarks on the GeometryInstance class in the RevitAPIDocs,
-  /// https://www.revitapidocs.com/2024/fe25b14f-5866-ca0f-a660-c157484c3a56.htm,
-  /// a family instance geometryElement should have a top-level geometry instance when the symbol
-  /// does not have modified geometry (the docs say that modified geometry will not have a geom instance,
-  /// however in my experience, all family instances have a top-level geom instance, but if the family instance
-  /// is modified, then the geom instance won't contain any geometry.)
-  ///
-  /// This remark also leads me to think that a family instance will not have top-level solids and geom instances.
-  /// We are logging cases where this is not true.
-  ///
-  /// Note: this is basically a geometry unpacker for all types of geometry
+  /// Sorts element geometry into solids, meshes, curves, polylines, points.
   /// </summary>
+  /// <remarks>
+  /// GeometryInstances are processed via GetSymbolGeometry() with accumulated transforms,
+  /// keeping meshes in symbol space and avoiding double transforms.
+  /// </remarks>
   private void SortGeometry(
     DB.Element element,
     GeometryCollections collections,
     DB.GeometryElement geom,
-    DB.Transform? worldToLocal
+    DB.Transform? accumulatedTransform
   )
   {
     foreach (DB.GeometryObject geomObj in geom)
@@ -359,56 +339,62 @@ public sealed class DisplayValueExtractor
       switch (geomObj)
       {
         case DB.Solid solid:
-          // skip invalid solid
           if (solid.Faces.Size == 0)
           {
             continue;
           }
 
-          if (worldToLocal is not null)
+          if (accumulatedTransform != null)
           {
-            solid = DB.SolidUtils.CreateTransformed(solid, worldToLocal);
+            // apply transform to bring solid into document/world space
+            // only apply once to avoid double-transform bugs
+            solid = DB.SolidUtils.CreateTransformed(solid, accumulatedTransform);
           }
+
           collections.Solids.Add(solid);
           break;
 
         case DB.Mesh mesh:
-          if (worldToLocal is not null)
+          if (accumulatedTransform != null)
           {
-            mesh = mesh.get_Transformed(worldToLocal);
+            // apply accumulated transform to mesh
+            // prevents geometry from being incorrectly transformed later [Ref: CNX-2875]
+            mesh = mesh.get_Transformed(accumulatedTransform);
           }
+
           collections.Meshes.Add(mesh);
           break;
 
-        // curves, polylines, and points are transformed to world space in ProcessGeometryCollections,
-        // not here, because they cannot be proxified like meshes.
         case DB.Curve curve:
+          // curves are stored as-is; transforms are applied later in ProcessGeometryCollections
           collections.Curves.Add(curve);
           break;
 
         case DB.PolyLine polyline:
+          // polylines also handled later during display value processing
           collections.Polylines.Add(polyline);
           break;
 
         case DB.Point point:
+          // points remain in local space; transformed later if needed
           collections.Points.Add(point);
           break;
 
         case DB.GeometryInstance instance:
-          // element transforms should not be carried down into nested geometryInstances.
-          // Nested geomInstances should have their geom retrieved with GetInstanceGeom, not GetSymbolGeom
-          if (worldToLocal == null) //see remark on method for why this is safe to do...
-          {
-            SortGeometry(element, collections, instance.GetInstanceGeometry(), null);
-          }
-          else
-          {
-            SortGeometry(element, collections, instance.GetSymbolGeometry(), null);
-          }
+          // GeometryInstance.Transform: symbol → parent coordinate system
+          // multiply with accumulatedTransform to handle nested instances
+          var instanceTransform = instance.Transform;
+          var nextTransform =
+            accumulatedTransform != null ? accumulatedTransform.Multiply(instanceTransform) : instanceTransform;
+
+          // always use symbol geometry, never GetInstanceGeometry() [Ref: CNX-2875]
+          SortGeometry(element, collections, instance.GetSymbolGeometry(), nextTransform);
           break;
 
         case DB.GeometryElement geometryElement:
-          SortGeometry(element, collections, geometryElement, null);
+          // raw GeometryElement: it has no transform of its own
+          // pass accumulatedTransform from parent if present
+          SortGeometry(element, collections, geometryElement, accumulatedTransform);
           break;
       }
     }
@@ -500,6 +486,26 @@ public sealed class DisplayValueExtractor
       return currentOptions;
     }
 
+    // cable trays (and fittings) are MEP system families whose geometry detail is effectively view-driven.
+    // So, we've seen that, Options.DetailLevel is ignored by get_Geometry() for these categories unless a View is
+    // explicitly supplied, and Revit will always return a medium-detail representation otherwise [Ref: CNX-2735]
+    // We force extraction through the active view here (if there is one!)
+    if (
+      elementBuiltInCategory == DB.BuiltInCategory.OST_CableTray
+      || elementBuiltInCategory == DB.BuiltInCategory.OST_CableTrayFitting
+    )
+    {
+      try
+      {
+        return new DB.Options { View = _converterSettings.Current.Document.NotNull().ActiveView };
+      }
+      catch (Exception ex) when (!ex.IsFatal())
+      {
+        // linked docs or invalid view context – fall back to non-view-specific options
+        return currentOptions;
+      }
+    }
+
     // NOTE: On steel elements. This is an incomplete solution.
     // If steel element proxies will be sucked in via category selection, and they are not visible in the current view, they will not be extracted out.
     // I'm inclined to go with this as a semi-permanent limitation. See:

Sdk/Speckle.Connectors.Common/Caching/ISendConversionCache.cs

@@ -14,6 +14,7 @@ namespace Speckle.Connectors.Common.Caching;
 public interface ISendConversionCache
 {
   void StoreSendResult(string projectId, IReadOnlyDictionary<Id, ObjectReference> convertedReferences);
+  void AppendSendResult(string projectId, string applicationId, ObjectReference convertedReference);
 
   /// <summary>
   /// <para>Call this method whenever you need to invalidate a set of objects that have changed in the host app.</para>

Sdk/Speckle.Connectors.Common/Caching/NullSendConversionCache.cs

@@ -11,6 +11,8 @@ public class NullSendConversionCache : ISendConversionCache
 {
   public void StoreSendResult(string projectId, IReadOnlyDictionary<Id, ObjectReference> convertedReferences) { }
 
+  public void AppendSendResult(string projectId, string applicationId, ObjectReference convertedReference) { }
+
   public void EvictObjects(IEnumerable<string> objectIds) { }
 
   public void ClearCache() { }

Sdk/Speckle.Connectors.Common/Caching/SendConversionCache.cs

@@ -17,6 +17,11 @@ public class SendConversionCache : ISendConversionCache
     }
   }
 
+  public void AppendSendResult(string projectId, string applicationId, ObjectReference convertedReference)
+  {
+    Cache[(applicationId, projectId)] = convertedReference;
+  }
+
   /// <inheritdoc/>
   public void EvictObjects(IEnumerable<string> objectIds) =>
     Cache = Cache