Fix kcl singleton

rust/kcl-lib/src/engine/mod.rs

@@ -228,6 +228,10 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         cmd: &ModelingCmd,
     ) -> Result<(), crate::errors::KclError> {
         // In isolated mode, we don't send the command to the engine.
+        //
+        // Note: It's important to allow commands through for the mock engine
+        // because it needs the commands to build the artifact graph in sketch
+        // mode.
         if self.execution_kind().await.is_isolated() {
             return Ok(());
         }
@@ -253,6 +257,10 @@ pub trait EngineManager: std::fmt::Debug + Send + Sync + 'static {
         cmd: &ModelingCmd,
     ) -> Result<(), crate::errors::KclError> {
         // In isolated mode, we don't send the command to the engine.
+        //
+        // Note: It's important to allow commands through for the mock engine
+        // because it needs the commands to build the artifact graph in sketch
+        // mode.
         if self.execution_kind().await.is_isolated() {
             return Ok(());
         }

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

@@ -488,6 +488,11 @@ impl ArtifactGraph {
     pub fn len(&self) -> usize {
         self.map.len()
     }
+
+    #[cfg(test)]
+    pub(crate) fn iter(&self) -> impl Iterator<Item = (&ArtifactId, &Artifact)> {
+        self.map.iter()
+    }
 }
 
 pub(super) fn build_artifact_graph(
@@ -622,10 +627,7 @@ fn artifacts_to_update(
     let uuid = artifact_command.cmd_id;
     let id = ArtifactId::new(uuid);
 
-    let Some(response) = responses.get(&uuid) else {
-        // Response not found or not successful.
-        return Ok(Vec::new());
-    };
+    let response = responses.get(&uuid);
 
     let cmd = &artifact_command.command;
 
@@ -757,7 +759,7 @@ fn artifacts_to_update(
                 new_path.seg_ids = vec![id];
                 return_arr.push(Artifact::Path(new_path));
             }
-            if let OkModelingCmdResponse::ClosePath(close_path) = response {
+            if let Some(OkModelingCmdResponse::ClosePath(close_path)) = response {
                 return_arr.push(Artifact::Solid2d(Solid2d {
                     id: close_path.face_id.into(),
                     path_id,
@@ -800,7 +802,7 @@ fn artifacts_to_update(
             return Ok(return_arr);
         }
         ModelingCmd::Loft(loft_cmd) => {
-            let OkModelingCmdResponse::Loft(_) = response else {
+            let Some(OkModelingCmdResponse::Loft(_)) = response else {
                 return Ok(Vec::new());
             };
             let mut return_arr = Vec::new();
@@ -830,7 +832,7 @@ fn artifacts_to_update(
             return Ok(return_arr);
         }
         ModelingCmd::Solid3dGetExtrusionFaceInfo(_) => {
-            let OkModelingCmdResponse::Solid3dGetExtrusionFaceInfo(face_info) = response else {
+            let Some(OkModelingCmdResponse::Solid3dGetExtrusionFaceInfo(face_info)) = response else {
                 return Ok(Vec::new());
             };
             let mut return_arr = Vec::new();
@@ -954,6 +956,11 @@ fn artifacts_to_update(
                 ModelingCmd::Solid3dGetOppositeEdge(_) => SweepEdgeSubType::Opposite,
                 _ => unreachable!(),
             };
+            // We need a response to continue.  If we're in sketch mode doing
+            // mock execution, we won't have one.
+            if response.is_none() {
+                return Ok(Vec::new());
+            }
             let face_id = ArtifactId::new(*face_id);
             let edge_id = ArtifactId::new(*edge_id);
             let Some(Artifact::Wall(wall)) = artifacts.get(&face_id) else {
@@ -969,7 +976,7 @@ fn artifacts_to_update(
                 return Ok(Vec::new());
             };
             let response_edge_id = match response {
-                OkModelingCmdResponse::Solid3dGetNextAdjacentEdge(r) => {
+                Some(OkModelingCmdResponse::Solid3dGetNextAdjacentEdge(r)) => {
                     let Some(edge_id) = r.edge else {
                         return Err(KclError::Internal(KclErrorDetails {
                             message:format!(
@@ -980,7 +987,7 @@ fn artifacts_to_update(
                     };
                     edge_id.into()
                 }
-                OkModelingCmdResponse::Solid3dGetOppositeEdge(r) => r.edge.into(),
+                Some(OkModelingCmdResponse::Solid3dGetOppositeEdge(r)) => r.edge.into(),
                 _ => {
                     return Err(KclError::Internal(KclErrorDetails {
                         message:format!(

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

@@ -5,6 +5,7 @@ use std::sync::Arc;
 use itertools::{EitherOrBoth, Itertools};
 use tokio::sync::RwLock;
 
+use super::IdGenerator;
 use crate::{
     execution::{annotations, memory::Stack, EnvironmentRef, ExecState, ExecutorSettings},
     parsing::ast::types::{Annotation, Node, Program},
@@ -14,8 +15,10 @@ use crate::{
 lazy_static::lazy_static! {
     /// A static mutable lock for updating the last successful execution state for the cache.
     static ref OLD_AST: Arc<RwLock<Option<OldAstState>>> = Default::default();
-    // The last successful run's memory. Not cleared after an unssuccessful run.
+    // The last successful run's memory. Not cleared after an unsuccessful run.
     static ref PREV_MEMORY: Arc<RwLock<Option<Stack>>> = Default::default();
+    /// The ID generator for mock execution.
+    static ref MOCK_ID_GENERATOR: Arc<RwLock<Option<IdGenerator>>> = Default::default();
 }
 
 /// Read the old ast memory from the lock.
@@ -49,6 +52,21 @@ pub async fn clear_mem_cache() {
     *old_mem = None;
 }
 
+pub(crate) async fn read_mock_ids() -> Option<IdGenerator> {
+    let cache = MOCK_ID_GENERATOR.read().await;
+    cache.clone()
+}
+
+pub(super) async fn write_mock_ids(id_gen: IdGenerator) {
+    let mut cache = MOCK_ID_GENERATOR.write().await;
+    *cache = Some(id_gen);
+}
+
+pub async fn clear_mock_ids() {
+    let mut cache = MOCK_ID_GENERATOR.write().await;
+    *cache = None;
+}
+
 /// Information for the caching an AST and smartly re-executing it if we can.
 #[derive(Debug, Clone)]
 pub struct CacheInformation<'a> {

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

@@ -7,7 +7,7 @@ pub use artifact::{
     Artifact, ArtifactCommand, ArtifactGraph, ArtifactId, CodeRef, StartSketchOnFace, StartSketchOnPlane,
 };
 use cache::OldAstState;
-pub use cache::{bust_cache, clear_mem_cache};
+pub use cache::{bust_cache, clear_mem_cache, clear_mock_ids};
 pub use cad_op::Operation;
 pub use geometry::*;
 pub(crate) use import::{
@@ -518,7 +518,9 @@ impl ExecutorContext {
     ) -> Result<ExecOutcome, KclErrorWithOutputs> {
         assert!(self.is_mock());
 
-        let mut exec_state = ExecState::new(&self.settings);
+        let mut id_generator = cache::read_mock_ids().await.unwrap_or_default();
+        id_generator.next_id = 0;
+        let mut exec_state = ExecState::with_ids(&self.settings, id_generator);
         if use_prev_memory {
             match cache::read_old_memory().await {
                 Some(mem) => *exec_state.mut_stack() = mem,
@@ -534,6 +536,10 @@ impl ExecutorContext {
 
         let result = self.inner_run(&program, &mut exec_state, true).await?;
 
+        // Mock execution has its own ID generator.  Save it so that multiple executions get the
+        // same IDs.
+        cache::write_mock_ids(exec_state.global.id_generator.clone()).await;
+
         // Restore any temporary variables, then save any newly created variables back to
         // memory in case another run wants to use them. Note this is just saved to the preserved
         // memory, not to the exec_state which is not cached for mock execution.
@@ -1990,4 +1996,22 @@ let w = f() + f()
         let result = ctx2.run_mock(program2, true).await.unwrap();
         assert_eq!(result.variables.get("z").unwrap().as_f64().unwrap(), 3.0);
     }
+
+    #[tokio::test(flavor = "multi_thread")]
+    async fn mock_has_stable_ids() {
+        let ctx = ExecutorContext::new_mock().await;
+        let code = "sk = startSketchOn(XY)
+        |> startProfileAt([0, 0], %)";
+        let program = crate::Program::parse_no_errs(code).unwrap();
+        let result = ctx.run_mock(program, false).await.unwrap();
+        let ids = result.artifact_graph.iter().map(|(k, _)| *k).collect::<Vec<_>>();
+        assert!(!ids.is_empty(), "IDs should not be empty");
+
+        let ctx2 = ExecutorContext::new_mock().await;
+        let program2 = crate::Program::parse_no_errs(code).unwrap();
+        let result = ctx2.run_mock(program2, false).await.unwrap();
+        let ids2 = result.artifact_graph.iter().map(|(k, _)| *k).collect::<Vec<_>>();
+
+        assert_eq!(ids, ids2, "Generated IDs should match");
+    }
 }

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

@@ -74,8 +74,12 @@ pub(super) struct ModuleState {
 
 impl ExecState {
     pub fn new(exec_settings: &ExecutorSettings) -> Self {
+        Self::with_ids(exec_settings, IdGenerator::default())
+    }
+
+    pub fn with_ids(exec_settings: &ExecutorSettings, id_generator: IdGenerator) -> Self {
         ExecState {
-            global: GlobalState::new(exec_settings),
+            global: GlobalState::new(exec_settings, id_generator),
             mod_local: ModuleState::new(exec_settings, None, ProgramMemory::new()),
         }
     }
@@ -86,8 +90,7 @@ impl ExecState {
         // This is for the front end to keep track of the ids.
         id_generator.next_id = 0;
 
-        let mut global = GlobalState::new(exec_settings);
-        global.id_generator = id_generator;
+        let global = GlobalState::new(exec_settings, id_generator);
 
         *self = ExecState {
             global,
@@ -145,8 +148,8 @@ impl ExecState {
                 .map(|(k, v)| (k.clone(), v.clone()))
                 .collect(),
             operations: Default::default(),
-            artifact_commands: Default::default(),
-            artifact_graph: Default::default(),
+            artifact_commands: self.global.artifact_commands,
+            artifact_graph: self.global.artifact_graph,
             errors: self.global.errors,
             filenames: Default::default(),
         }
@@ -239,9 +242,9 @@ impl ExecState {
 }
 
 impl GlobalState {
-    fn new(settings: &ExecutorSettings) -> Self {
+    fn new(settings: &ExecutorSettings, id_generator: IdGenerator) -> Self {
         let mut global = GlobalState {
-            id_generator: Default::default(),
+            id_generator,
             path_to_source_id: Default::default(),
             module_infos: Default::default(),
             artifacts: Default::default(),

rust/kcl-lib/src/lib.rs

@@ -86,7 +86,8 @@ pub use errors::{
     CompilationError, ConnectionError, ExecError, KclError, KclErrorWithOutputs, Report, ReportWithOutputs,
 };
 pub use execution::{
-    bust_cache, clear_mem_cache, ExecOutcome, ExecState, ExecutorContext, ExecutorSettings, MetaSettings, Point2d,
+    bust_cache, clear_mem_cache, clear_mock_ids, ExecOutcome, ExecState, ExecutorContext, ExecutorSettings,
+    MetaSettings, Point2d,
 };
 pub use lsp::{
     copilot::Backend as CopilotLspBackend,

rust/kcl-lib/tests/basic_fillet_cube_next_adjacent/artifact_graph_flowchart.snap

@@ -1,5 +1,5 @@
 ---
-source: kcl/src/simulation_tests.rs
+source: kcl-lib/src/simulation_tests.rs
 description: Artifact graph flowchart basic_fillet_cube_next_adjacent.kcl
 extension: md
 snapshot_kind: binary

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

@@ -24,6 +24,7 @@ flowchart LR
   20["SweepEdge Adjacent"]
   21["SweepEdge Opposite"]
   22["SweepEdge Adjacent"]
+  23["EdgeCut Fillet<br>[238, 294, 0]"]
   1 --- 2
   2 --- 3
   2 --- 4
@@ -57,4 +58,5 @@ flowchart LR
   8 --- 20
   8 --- 21
   8 --- 22
+  16 <--x 23
 ```

rust/kcl-lib/tests/basic_fillet_cube_previous_adjacent/artifact_graph_flowchart.snap

@@ -1,5 +1,5 @@
 ---
-source: kcl/src/simulation_tests.rs
+source: kcl-lib/src/simulation_tests.rs
 description: Artifact graph flowchart basic_fillet_cube_previous_adjacent.kcl
 extension: md
 snapshot_kind: binary

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

@@ -24,6 +24,7 @@ flowchart LR
   20["SweepEdge Adjacent"]
   21["SweepEdge Opposite"]
   22["SweepEdge Adjacent"]
+  23["EdgeCut Fillet<br>[238, 298, 0]"]
   1 --- 2
   2 --- 3
   2 --- 4
@@ -57,4 +58,5 @@ flowchart LR
   8 --- 20
   8 --- 21
   8 --- 22
+  18 <--x 23
 ```

rust/kcl-lib/tests/kcl_samples/multi-axis-robot/artifact_graph_flowchart.snap

@@ -1,5 +1,5 @@
 ---
-source: kcl/src/simulation_tests.rs
+source: kcl-lib/src/simulation_tests.rs
 description: Artifact graph flowchart multi-axis-robot.kcl
 extension: md
 snapshot_kind: binary

rust/kcl-wasm-lib/src/wasm.rs

@@ -5,8 +5,8 @@ use std::sync::Arc;
 use futures::stream::TryStreamExt;
 use gloo_utils::format::JsValueSerdeExt;
 use kcl_lib::{
-    bust_cache, clear_mem_cache, exec::IdGenerator, pretty::NumericSuffix, CoreDump, EngineManager, ModuleId, Point2d,
-    Program,
+    bust_cache, clear_mem_cache, clear_mock_ids, exec::IdGenerator, pretty::NumericSuffix, CoreDump, EngineManager,
+    ModuleId, Point2d, Program,
 };
 use tower_lsp::{LspService, Server};
 use wasm_bindgen::prelude::*;
@@ -20,6 +20,7 @@ pub async fn clear_scene_and_bust_cache(
 
     bust_cache().await;
     clear_mem_cache().await;
+    clear_mock_ids().await;
 
     let engine = kcl_lib::wasm_engine::EngineConnection::new(engine_manager)
         .await

src/lang/KclSingleton.ts

@@ -476,12 +476,12 @@ export class KclManager {
     })
 
     this._logs = logs
-    this._execState = execState
-    this._variables = execState.variables
+    this.execState = execState
     if (!errors.length) {
       this.lastSuccessfulVariables = execState.variables
       this.lastSuccessfulOperations = execState.operations
     }
+    this.engineCommandManager.updateArtifactGraph(execState.artifactGraph)
   }
   cancelAllExecutions() {
     this._cancelTokens.forEach((_, key) => {

src/lang/wasm.ts

@@ -320,17 +320,10 @@ function execStateFromRust(
   execOutcome: RustExecOutcome,
   program: Node<Program>
 ): ExecState {
-  const artifactGraph = rustArtifactGraphToMap(execOutcome.artifactGraph)
-  // We haven't ported pathToNode logic to Rust yet, so we need to fill it in.
-  for (const [id, artifact] of artifactGraph) {
-    if (!artifact) continue
-    if (!('codeRef' in artifact)) continue
-    const pathToNode = getNodePathFromSourceRange(
-      program,
-      sourceRangeFromRust(artifact.codeRef.range)
-    )
-    artifact.codeRef.pathToNode = pathToNode
-  }
+  const artifactGraph = artifactGraphFromRust(
+    execOutcome.artifactGraph,
+    program
+  )
 
   return {
     variables: execOutcome.variables,
@@ -342,29 +335,30 @@ function execStateFromRust(
   }
 }
 
-function mockExecStateFromRust(execOutcome: RustExecOutcome): ExecState {
-  return {
-    variables: execOutcome.variables,
-    operations: execOutcome.operations,
-    artifactCommands: execOutcome.artifactCommands,
-    artifactGraph: new Map<ArtifactId, Artifact>(),
-    errors: execOutcome.errors,
-    filenames: execOutcome.filenames,
-  }
-}
-
 export type ArtifactGraph = Map<ArtifactId, Artifact>
 
-function rustArtifactGraphToMap(
-  rustArtifactGraph: RustArtifactGraph
+function artifactGraphFromRust(
+  rustArtifactGraph: RustArtifactGraph,
+  program: Node<Program>
 ): ArtifactGraph {
-  const map = new Map<ArtifactId, Artifact>()
+  const artifactGraph = new Map<ArtifactId, Artifact>()
+  // Convert to a Map.
   for (const [id, artifact] of Object.entries(rustArtifactGraph.map)) {
     if (!artifact) continue
-    map.set(id, artifact)
+    artifactGraph.set(id, artifact)
   }
 
-  return map
+  // We haven't ported pathToNode logic to Rust yet, so we need to fill it in.
+  for (const [id, artifact] of artifactGraph) {
+    if (!artifact) continue
+    if (!('codeRef' in artifact)) continue
+    const pathToNode = getNodePathFromSourceRange(
+      program,
+      sourceRangeFromRust(artifact.codeRef.range)
+    )
+    artifact.codeRef.pathToNode = pathToNode
+  }
+  return artifactGraph
 }
 
 export function defaultArtifactGraph(): ArtifactGraph {
@@ -427,9 +421,9 @@ export const executeMock = async (
       usePrevMemory,
       fileSystemManager
     )
-    return mockExecStateFromRust(execOutcome)
+    return execStateFromRust(execOutcome, node)
   } catch (e: any) {
-    return Promise.reject(errFromErrWithOutputs(e))
+    return Promise.reject(errFromErrWithOutputs(e, node))
   }
 }
 
@@ -454,7 +448,7 @@ export const executeWithEngine = async (
     )
     return execStateFromRust(execOutcome, node)
   } catch (e: any) {
-    return Promise.reject(errFromErrWithOutputs(e))
+    return Promise.reject(errFromErrWithOutputs(e, node))
   }
 }
 
@@ -471,7 +465,7 @@ const jsAppSettings = async () => {
   return jsAppSettings
 }
 
-const errFromErrWithOutputs = (e: any): KCLError => {
+const errFromErrWithOutputs = (e: any, program: Node<Program>): KCLError => {
   const parsed: KclErrorWithOutputs = JSON.parse(e.toString())
   return new KCLError(
     parsed.error.kind,
@@ -479,7 +473,7 @@ const errFromErrWithOutputs = (e: any): KCLError => {
     firstSourceRange(parsed.error),
     parsed.operations,
     parsed.artifactCommands,
-    rustArtifactGraphToMap(parsed.artifactGraph),
+    artifactGraphFromRust(parsed.artifactGraph, program),
     parsed.filenames
   )
 }