Artifact graph recognizes inner faces (#7805)

Consider a 2D sketch with a hole. Such a sketch has both outer edges and inner edges.

When this sketch is extruded, the resulting solid will have both outer faces and inner faces.

In #7753 I fixed a bug where sketches weren't tracking their inner edges, and solids weren't tracking their inner faces. But this only affected the KCL runtime. I forgot to update the artifact graph to handle these faces. This PR ensures the artifact graph still builds even if a solid has inner faces. For example, this solid couldn't have an artifact graph before:

<img width="1632" height="1376" alt="inner face" src="https://github.com/user-attachments/assets/7af80f76-4e29-4be9-8bc9-45f911590269" />

Author: adamchalmers

rust/kcl-lib/src/execution/artifact.rs

@@ -128,6 +128,13 @@ pub struct Path {
     /// this can be used as input for another composite solid.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     pub composite_solid_id: Option<ArtifactId>,
+    /// The hole, if any, from a subtract2d() call.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub inner_path_id: Option<ArtifactId>,
+    /// The `Path` that this is a hole of, if any. The inverse link of
+    /// `inner_path_id`.
+    #[serde(default, skip_serializing_if = "Option::is_none")]
+    pub outer_path_id: Option<ArtifactId>,
 }
 
 #[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)]
@@ -443,6 +450,8 @@ impl Path {
         merge_ids(&mut self.seg_ids, new.seg_ids);
         merge_opt_id(&mut self.solid2d_id, new.solid2d_id);
         merge_opt_id(&mut self.composite_solid_id, new.composite_solid_id);
+        merge_opt_id(&mut self.inner_path_id, new.inner_path_id);
+        merge_opt_id(&mut self.outer_path_id, new.outer_path_id);
 
         None
     }
@@ -805,6 +814,8 @@ fn artifacts_to_update(
                 solid2d_id: None,
                 code_ref,
                 composite_solid_id: None,
+                inner_path_id: None,
+                outer_path_id: None,
             }));
             let plane = artifacts.get(&ArtifactId::new(*current_plane_id));
             if let Some(Artifact::Plane(plane)) = plane {
@@ -924,6 +935,8 @@ fn artifacts_to_update(
                         solid2d_id: None,
                         code_ref: code_ref.clone(),
                         composite_solid_id: None,
+                        inner_path_id: None,
+                        outer_path_id: None,
                     }
                 };
 
@@ -1030,14 +1043,20 @@ fn artifacts_to_update(
                     continue;
                 };
                 last_path = Some(path);
-                let path_sweep_id = path.sweep_id.ok_or_else(|| {
-                    KclError::new_internal(KclErrorDetails::new(
+                let Some(path_sweep_id) = path.sweep_id else {
+                    // If the path doesn't have a sweep ID, check if it's a
+                    // hole.
+                    if path.outer_path_id.is_some() {
+                        // This is a hole.
+                        continue;
+                    }
+                    return Err(KclError::new_internal(KclErrorDetails::new(
                         format!(
-                            "Expected a sweep ID on the path when processing Solid3dGetExtrusionFaceInfo command, but we have none: {id:?}, {path:?}"
+                            "Expected a sweep ID on the path when processing Solid3dGetExtrusionFaceInfo command, but we have none:\n{id:#?}\n{path:#?}"
                         ),
                         vec![range],
-                    ))
-                })?;
+                    )));
+                };
                 let extra_artifact = exec_artifacts.values().find(|a| {
                     if let Artifact::StartSketchOnFace(s) = a {
                         s.face_id == face_id
@@ -1084,14 +1103,20 @@ fn artifacts_to_update(
                     let Some(face_id) = face.face_id.map(ArtifactId::new) else {
                         continue;
                     };
-                    let path_sweep_id = path.sweep_id.ok_or_else(|| {
-                        KclError::new_internal(KclErrorDetails::new(
+                    let Some(path_sweep_id) = path.sweep_id else {
+                        // If the path doesn't have a sweep ID, check if it's a
+                        // hole.
+                        if path.outer_path_id.is_some() {
+                            // This is a hole.
+                            continue;
+                        }
+                        return Err(KclError::new_internal(KclErrorDetails::new(
                             format!(
-                                "Expected a sweep ID on the path when processing last path's Solid3dGetExtrusionFaceInfo command, but we have none: {id:?}, {path:?}"
+                                "Expected a sweep ID on the path when processing last path's Solid3dGetExtrusionFaceInfo command, but we have none:\n{id:#?}\n{path:#?}"
                             ),
                             vec![range],
-                        ))
-                    })?;
+                        )));
+                    };
                     let extra_artifact = exec_artifacts.values().find(|a| {
                         if let Artifact::StartSketchOnFace(s) = a {
                             s.face_id == face_id
@@ -1267,6 +1292,24 @@ fn artifacts_to_update(
             // the helix here, but it's not useful right now.
             return Ok(return_arr);
         }
+        ModelingCmd::Solid2dAddHole(solid2d_add_hole) => {
+            let mut return_arr = Vec::new();
+            // Add the hole to the outer.
+            let outer_path = artifacts.get(&ArtifactId::new(solid2d_add_hole.object_id));
+            if let Some(Artifact::Path(path)) = outer_path {
+                let mut new_path = path.clone();
+                new_path.inner_path_id = Some(ArtifactId::new(solid2d_add_hole.hole_id));
+                return_arr.push(Artifact::Path(new_path));
+            }
+            // Add the outer to the hole.
+            let inner_solid2d = artifacts.get(&ArtifactId::new(solid2d_add_hole.hole_id));
+            if let Some(Artifact::Path(path)) = inner_solid2d {
+                let mut new_path = path.clone();
+                new_path.outer_path_id = Some(ArtifactId::new(solid2d_add_hole.object_id));
+                return_arr.push(Artifact::Path(new_path));
+            }
+            return Ok(return_arr);
+        }
         ModelingCmd::BooleanIntersection(_) | ModelingCmd::BooleanSubtract(_) | ModelingCmd::BooleanUnion(_) => {
             let (sub_type, solid_ids, tool_ids) = match cmd {
                 ModelingCmd::BooleanIntersection(intersection) => {

rust/kcl-lib/src/execution/artifact/mermaid_tests.rs

@@ -73,7 +73,16 @@ impl Artifact {
                 ids
             }
             Artifact::Plane(_) => Vec::new(),
-            Artifact::Path(a) => vec![a.plane_id],
+            Artifact::Path(a) => {
+                let mut ids = vec![a.plane_id];
+                if let Some(inner_path_id) = a.inner_path_id {
+                    ids.push(inner_path_id);
+                }
+                if let Some(outer_path_id) = a.outer_path_id {
+                    ids.push(outer_path_id);
+                }
+                ids
+            }
             Artifact::Segment(a) => vec![a.path_id],
             Artifact::Solid2d(a) => vec![a.path_id],
             Artifact::StartSketchOnFace(a) => vec![a.face_id],
@@ -103,7 +112,8 @@ impl Artifact {
             }
             Artifact::Plane(a) => a.path_ids.clone(),
             Artifact::Path(a) => {
-                // Note: Don't include these since they're parents: plane_id.
+                // Note: Don't include these since they're parents: plane_id,
+                // inner_path_id, outer_path_id.
                 let mut ids = a.seg_ids.clone();
                 if let Some(sweep_id) = a.sweep_id {
                     ids.push(sweep_id);

rust/kcl-lib/src/simulation_tests.rs

@@ -3817,3 +3817,24 @@ mod elliptic_curve_inches_regression {
         super::execute(TEST_NAME, true).await
     }
 }
+mod tag_inner_face {
+    const TEST_NAME: &str = "tag_inner_face";
+
+    /// Test parsing KCL.
+    #[test]
+    fn parse() {
+        super::parse(TEST_NAME)
+    }
+
+    /// Test that parsing and unparsing KCL produces the original KCL input.
+    #[tokio::test(flavor = "multi_thread")]
+    async fn unparse() {
+        super::unparse(TEST_NAME).await
+    }
+
+    /// Test that KCL is executed correctly.
+    #[tokio::test(flavor = "multi_thread")]
+    async fn kcl_test_execute() {
+        super::execute(TEST_NAME, true).await
+    }
+}

rust/kcl-lib/tests/fillet-and-shell/artifact_graph_flowchart.snap.md

@@ -239,6 +239,7 @@ flowchart LR
   31 --- 36
   33 --- 34
   33 --- 35
+  36 --- 33
   33 ---- 39
   34 --- 40
   34 x--> 41
@@ -259,6 +260,7 @@ flowchart LR
   45 --- 50
   47 --- 48
   47 --- 49
+  50 --- 47
   47 ---- 53
   48 --- 54
   48 x--> 55
@@ -279,6 +281,7 @@ flowchart LR
   59 --- 64
   61 --- 62
   61 --- 63
+  64 --- 61
   61 ---- 67
   62 --- 68
   62 x--> 69
@@ -299,6 +302,7 @@ flowchart LR
   73 --- 78
   75 --- 76
   75 --- 77
+  78 --- 75
   75 ---- 81
   76 --- 82
   76 x--> 83

rust/kcl-lib/tests/flush_batch_on_end/artifact_graph_flowchart.snap.md

@@ -30,6 +30,7 @@ flowchart LR
   1 --- 5
   2 --- 3
   2 --- 4
+  5 --- 2
   2 ---- 8
   3 --- 9
   3 x--> 10

rust/kcl-lib/tests/i_shape/artifact_graph_flowchart.snap.md

@@ -211,6 +211,7 @@ flowchart LR
   2 --- 26
   2 --- 27
   2 --- 28
+  30 --- 2
   2 ---- 41
   3 --- 42
   3 x--> 66

rust/kcl-lib/tests/import_async/artifact_graph_flowchart.snap.md

@@ -98,6 +98,9 @@ flowchart LR
   2 --- 7
   2 --- 8
   2 --- 9
+  2 <--x 11
+  2 <--x 19
+  27 --- 2
   10 --- 11
   10 <--x 45
   11 --- 12

rust/kcl-lib/tests/involute_circular_units/artifact_graph_flowchart.snap.md

@@ -68,6 +68,7 @@ flowchart LR
   2 --- 8
   2 --- 9
   2 --- 10
+  11 --- 2
   2 ---- 14
   3 --- 15
   3 x--> 21

rust/kcl-lib/tests/multi_target_csg/artifact_graph_flowchart.snap.md

@@ -201,6 +201,7 @@ flowchart LR
   2 --- 10
   2 --- 11
   2 --- 12
+  13 --- 2
   2 ---- 16
   3 --- 24
   3 x--> 25

rust/kcl-lib/tests/sketch_on_face_union/artifact_graph_flowchart.snap.md

@@ -193,6 +193,8 @@ flowchart LR
   2 --- 6
   2 --- 7
   2 --- 8
+  2 <--x 9
+  15 --- 2
   2 ---- 21
   2 --- 82
   3 --- 26