KCL refactor: Solid should not have a Sketch field

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

@@ -46,6 +46,17 @@ impl<'a> StatementKind<'a> {
     }
 }
 
+fn tags_object_from_map(tags: &IndexMap<String, TagIdentifier>, source_range: SourceRange) -> KclValue {
+    KclValue::Object {
+        meta: vec![Metadata { source_range }],
+        value: tags
+            .iter()
+            .map(|(k, tag)| (k.to_owned(), KclValue::TagIdentifier(Box::new(tag.to_owned()))))
+            .collect(),
+        constrainable: false,
+    }
+}
+
 impl ExecutorContext {
     /// Returns true if importing the prelude should be skipped.
     async fn handle_annotations(
@@ -2016,10 +2027,18 @@ impl Node<MemberExpression> {
                     vec![self.clone().into()],
                 )))
             }
-            (KclValue::Solid { value }, Property::String(prop), false) if prop == "sketch" => Ok(KclValue::Sketch {
-                value: Box::new(value.sketch),
+            (KclValue::Solid { value }, Property::String(prop), false) if prop == "sketch" => {
+                let source_range = SourceRange::from(self.clone());
+                Ok(KclValue::Object {
+                    meta: vec![Metadata { source_range }],
+                    value: HashMap::from([(
+                        "tags".to_owned(),
+                        tags_object_from_map(&value.sketch.tags, source_range),
+                    )]),
+                    constrainable: false,
+                }
+                .continue_())
             }
-            .continue_()),
             (geometry @ KclValue::Solid { .. }, Property::String(prop), false) if prop == "tags" => {
                 // This is a common mistake.
                 Err(KclError::new_semantic(KclErrorDetails::new(
@@ -2030,18 +2049,9 @@ impl Node<MemberExpression> {
                     vec![self.clone().into()],
                 )))
             }
-            (KclValue::Sketch { value: sk }, Property::String(prop), false) if prop == "tags" => Ok(KclValue::Object {
-                meta: vec![Metadata {
-                    source_range: SourceRange::from(self.clone()),
-                }],
-                value: sk
-                    .tags
-                    .iter()
-                    .map(|(k, tag)| (k.to_owned(), KclValue::TagIdentifier(Box::new(tag.to_owned()))))
-                    .collect(),
-                constrainable: false,
+            (KclValue::Sketch { value: sk }, Property::String(prop), false) if prop == "tags" => {
+                Ok(tags_object_from_map(&sk.tags, SourceRange::from(self.clone())).continue_())
             }
-            .continue_()),
             (geometry @ (KclValue::Sketch { .. } | KclValue::Solid { .. }), Property::String(property), false) => {
                 Err(KclError::new_semantic(KclErrorDetails::new(
                     format!("Property `{property}` not found on {}", geometry.human_friendly_type()),

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

@@ -682,6 +682,62 @@ pub struct Sketch {
     pub is_closed: ProfileClosed,
 }
 
+#[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)]
+#[ts(export)]
+#[serde(tag = "type", rename_all = "camelCase")]
+pub struct SketchBase {
+    /// The id of the sketch (this will change when the engine's reference to it changes).
+    pub id: uuid::Uuid,
+    /// The paths in the sketch.
+    /// Only paths on the "outside" i.e. the perimeter.
+    /// Does not include paths "inside" the profile (for example, edges made by subtracting a profile)
+    pub paths: Vec<Path>,
+    /// Inner paths, resulting from subtract2d to carve profiles out of the sketch.
+    #[serde(default, skip_serializing_if = "Vec::is_empty")]
+    pub inner_paths: Vec<Path>,
+    /// What the sketch is on (can be a plane or a face).
+    pub on: SketchSurface,
+    /// The starting path.
+    pub start: BasePath,
+    /// Tag identifiers that have been declared in this sketch.
+    #[serde(default, skip_serializing_if = "IndexMap::is_empty")]
+    pub tags: IndexMap<String, TagIdentifier>,
+    /// The original id of the sketch. This stays the same even if the sketch is
+    /// is sketched on face etc.
+    pub artifact_id: ArtifactId,
+    #[ts(skip)]
+    pub original_id: uuid::Uuid,
+    /// If the sketch includes a mirror.
+    #[serde(skip)]
+    pub mirror: Option<uuid::Uuid>,
+    /// If the sketch is a clone of another sketch.
+    #[serde(skip)]
+    pub clone: Option<uuid::Uuid>,
+}
+
+impl From<Sketch> for SketchBase {
+    fn from(value: Sketch) -> Self {
+        Self {
+            id: value.id,
+            paths: value.paths,
+            inner_paths: value.inner_paths,
+            on: value.on,
+            start: value.start,
+            tags: value.tags,
+            artifact_id: value.artifact_id,
+            original_id: value.original_id,
+            mirror: value.mirror,
+            clone: value.clone,
+        }
+    }
+}
+
+impl From<&Sketch> for SketchBase {
+    fn from(value: &Sketch) -> Self {
+        value.clone().into()
+    }
+}
+
 impl ProfileClosed {
     #[expect(dead_code, reason = "it's not actually dead, it's called by serde")]
     fn explicitly() -> Self {
@@ -881,6 +937,63 @@ impl Sketch {
     }
 }
 
+impl SketchBase {
+    pub(crate) fn add_tag(
+        &mut self,
+        tag: NodeRef<'_, TagDeclarator>,
+        current_path: &Path,
+        exec_state: &ExecState,
+        surface: Option<&ExtrudeSurface>,
+    ) {
+        let mut tag_identifier: TagIdentifier = tag.into();
+        let base = current_path.get_base();
+        tag_identifier.info.push((
+            exec_state.stack().current_epoch(),
+            TagEngineInfo {
+                id: base.geo_meta.id,
+                sketch: self.id,
+                path: Some(current_path.clone()),
+                surface: surface.cloned(),
+            },
+        ));
+
+        self.tags.insert(tag.name.to_string(), tag_identifier);
+    }
+
+    pub(crate) fn merge_tags<'a>(&mut self, tags: impl Iterator<Item = &'a TagIdentifier>) {
+        for t in tags {
+            match self.tags.get_mut(&t.value) {
+                Some(id) => {
+                    id.merge_info(t);
+                }
+                None => {
+                    self.tags.insert(t.value.clone(), t.clone());
+                }
+            }
+        }
+    }
+}
+
+impl Sketch {
+    pub(crate) fn from_base_with_solid_context(base: &SketchBase, solid: &Solid) -> Self {
+        Self {
+            id: base.id,
+            paths: base.paths.clone(),
+            inner_paths: base.inner_paths.clone(),
+            on: base.on.clone(),
+            start: base.start.clone(),
+            tags: base.tags.clone(),
+            artifact_id: base.artifact_id,
+            original_id: base.original_id,
+            mirror: base.mirror,
+            clone: base.clone,
+            units: solid.units,
+            meta: solid.meta.clone(),
+            is_closed: ProfileClosed::Explicitly,
+        }
+    }
+}
+
 #[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)]
 #[ts(export)]
 #[serde(tag = "type", rename_all = "camelCase")]
@@ -892,7 +1005,7 @@ pub struct Solid {
     /// The extrude surfaces.
     pub value: Vec<ExtrudeSurface>,
     /// The sketch.
-    pub sketch: Sketch,
+    pub sketch: SketchBase,
     /// The id of the extrusion start cap
     pub start_cap_id: Option<uuid::Uuid>,
     /// The id of the extrusion end cap

rust/kcl-lib/src/std/clone.rs

@@ -14,7 +14,7 @@ use super::extrude::do_post_extrude;
 use crate::{
     errors::{KclError, KclErrorDetails},
     execution::{
-        ExecState, ExtrudeSurface, GeometryWithImportedGeometry, KclValue, ModelingCmdMeta, Sketch, Solid,
+        ExecState, ExtrudeSurface, GeometryWithImportedGeometry, KclValue, ModelingCmdMeta, Sketch, SketchBase, Solid,
         types::{PrimitiveType, RuntimeType},
     },
     parsing::ast::types::TagNode,
@@ -126,7 +126,7 @@ async fn fix_tags_and_references(
             solid.sketch.artifact_id = new_geometry_id.into();
             solid.sketch.clone = Some(old_geometry_id);
 
-            fix_sketch_tags_and_references(
+            fix_sketch_base_tags_and_references(
                 &mut solid.sketch,
                 &entity_id_map,
                 exec_state,
@@ -156,8 +156,9 @@ async fn fix_tags_and_references(
 
             // Do the after extrude things to update those ids, based on the new sketch
             // information.
+            let sketch = Sketch::from_base_with_solid_context(&solid.sketch, solid);
             let new_solid = do_post_extrude(
-                &solid.sketch,
+                &sketch,
                 new_geometry_id.into(),
                 solid.sectional,
                 &NamedCapTags {
@@ -310,6 +311,73 @@ async fn fix_sketch_tags_and_references(
     Ok(())
 }
 
+/// Fix the tags and references of a sketch base.
+async fn fix_sketch_base_tags_and_references(
+    new_sketch: &mut SketchBase,
+    entity_id_map: &HashMap<uuid::Uuid, uuid::Uuid>,
+    exec_state: &mut ExecState,
+    args: &Args,
+    surfaces: Option<Vec<ExtrudeSurface>>,
+) -> Result<()> {
+    // Fix the path references in the sketch.
+    for path in new_sketch.paths.as_mut_slice() {
+        if let Some(new_path_id) = entity_id_map.get(&path.get_id()) {
+            path.set_id(*new_path_id);
+        } else {
+            // We log on these because we might have already flushed and the id is no longer
+            // relevant since filleted or something.
+            crate::log::logln!("Failed to find new path id for old path id: {:?}", path.get_id());
+        }
+    }
+
+    // Map the surface tags to the new surface ids.
+    let mut surface_id_map: HashMap<String, &ExtrudeSurface> = HashMap::new();
+    let surfaces = surfaces.unwrap_or_default();
+    for surface in surfaces.iter() {
+        if let Some(tag) = surface.get_tag() {
+            surface_id_map.insert(tag.name.clone(), surface);
+        }
+    }
+
+    // Fix the tags
+    // This is annoying, in order to fix the tags we need to iterate over the paths again, but not
+    // mutable borrow the paths.
+    for path in new_sketch.paths.clone() {
+        // Check if this path has a tag.
+        if let Some(tag) = path.get_tag() {
+            let mut surface = None;
+            if let Some(found_surface) = surface_id_map.get(&tag.name) {
+                let mut new_surface = (*found_surface).clone();
+                let Some(new_face_id) = entity_id_map.get(&new_surface.face_id()).copied() else {
+                    return Err(KclError::new_engine(KclErrorDetails::new(
+                        format!(
+                            "Failed to find new face id for old face id: {:?}",
+                            new_surface.face_id()
+                        ),
+                        vec![args.source_range],
+                    )));
+                };
+                new_surface.set_face_id(new_face_id);
+                surface = Some(new_surface);
+            }
+
+            new_sketch.add_tag(&tag, &path, exec_state, surface.as_ref());
+        }
+    }
+
+    // Fix the base path.
+    if let Some(new_base_path) = entity_id_map.get(&new_sketch.start.geo_meta.id) {
+        new_sketch.start.geo_meta.id = *new_base_path;
+    } else {
+        crate::log::logln!(
+            "Failed to find new base path id for old base path id: {:?}",
+            new_sketch.start.geo_meta.id
+        );
+    }
+
+    Ok(())
+}
+
 // Return the named cap tags for the original solid.
 fn get_named_cap_tags(solid: &Solid) -> (Option<TagNode>, Option<TagNode>) {
     let mut start_tag = None;

rust/kcl-lib/src/std/extrude.rs

@@ -616,7 +616,7 @@ pub(crate) async fn do_post_extrude<'a>(
         meta: sketch.meta.clone(),
         units: sketch.units,
         sectional,
-        sketch,
+        sketch: crate::execution::SketchBase::from(sketch),
         start_cap_id,
         end_cap_id,
         edge_cuts: vec![],

rust/kcl-lib/tests/angled_line/program_memory.snap

@@ -61,7 +61,7 @@ description: Variables in memory after executing angled_line.kcl
         }
       ],
       "sketch": {
-        "type": "Sketch",
+        "type": "SketchBase",
         "id": "[uuid]",
         "paths": [
           {
@@ -227,8 +227,7 @@ description: Variables in memory after executing angled_line.kcl
           }
         },
         "artifactId": "[uuid]",
-        "originalId": "[uuid]",
-        "units": "mm"
+        "originalId": "[uuid]"
       },
       "startCapId": "[uuid]",
       "endCapId": "[uuid]",