Merge branch 'main' into paultag/remove-add

* start of seperating pending message from artifact map

* continue migration to sendCommandVersion2

* mostly massage types

* process artifact after the fact

* clean up

src/clientSideScene/sceneEntities.ts

@@ -2022,13 +2022,17 @@ export async function getFaceDetails(
       entity_id: entityId,
     },
   })
-  const faceInfo: Models['GetSketchModePlane_type'] = (
-    await engineCommandManager.sendSceneCommand({
-      type: 'modeling_cmd_req',
-      cmd_id: uuidv4(),
-      cmd: { type: 'get_sketch_mode_plane' },
-    })
-  )?.data?.data
+  const resp = await engineCommandManager.sendSceneCommand({
+    type: 'modeling_cmd_req',
+    cmd_id: uuidv4(),
+    cmd: { type: 'get_sketch_mode_plane' },
+  })
+  const faceInfo =
+    resp?.success &&
+    resp?.resp.type === 'modeling' &&
+    resp?.resp?.data?.modeling_response?.type === 'get_sketch_mode_plane'
+      ? resp?.resp?.data?.modeling_response.data
+      : ({} as Models['GetSketchModePlane_type'])
   await engineCommandManager.sendSceneCommand({
     type: 'modeling_cmd_req',
     cmd_id: uuidv4(),

src/components/ModelingMachineProvider.tsx

@@ -138,15 +138,23 @@ export const ModelingMachineProvider = ({
 
             sceneInfra.camControls.syncDirection = 'engineToClient'
 
-            const settings: Models['CameraSettings_type'] = (
-              await engineCommandManager.sendSceneCommand({
-                type: 'modeling_cmd_req',
-                cmd_id: uuidv4(),
-                cmd: {
-                  type: 'default_camera_get_settings',
-                },
-              })
-            )?.data?.data?.settings
+            const resp = await engineCommandManager.sendSceneCommand({
+              type: 'modeling_cmd_req',
+              cmd_id: uuidv4(),
+              cmd: {
+                type: 'default_camera_get_settings',
+              },
+            })
+
+            const settings =
+              resp &&
+              resp.success &&
+              resp.resp.type === 'modeling' &&
+              resp.resp.data.modeling_response.type ===
+                'default_camera_get_settings'
+                ? resp.resp.data.modeling_response.data.settings
+                : ({} as Models['DefaultCameraGetSettings_type']['settings'])
+
             if (settings.up.z !== 1) {
               // workaround for gimbal lock situation
               await engineCommandManager.sendSceneCommand({

src/editor/manager.ts

@@ -21,13 +21,10 @@ export const modelingMachineEvent = modelingMachineAnnotation.of(true)
 const setDiagnosticsAnnotation = Annotation.define<boolean>()
 export const setDiagnosticsEvent = setDiagnosticsAnnotation.of(true)
 
-function diagnosticIsEqual(d1: Diagnostic, d2: Diagnostic): boolean {
-  return d1.from === d2.from && d1.to === d2.to && d1.message === d2.message
-}
-
 export default class EditorManager {
   private _editorView: EditorView | null = null
   private _copilotEnabled: boolean = true
+  private _diagnostics: Diagnostic[] = []
 
   private _isShiftDown: boolean = false
   private _selectionRanges: Selections = {
@@ -118,6 +115,14 @@ export default class EditorManager {
     this.setDiagnostics([])
   }
 
+  addDiagnostics(diagnostics: Diagnostic[]): void {
+    if (!this._editorView) return
+    diagnostics.forEach((diagnostic) => {
+      this._diagnostics.push(diagnostic)
+    })
+    this.setDiagnostics(this._diagnostics)
+  }
+
   setDiagnostics(diagnostics: Diagnostic[]): void {
     if (!this._editorView) return
 
@@ -131,26 +136,6 @@ export default class EditorManager {
     })
   }
 
-  addDiagnostics(diagnostics: Diagnostic[]): void {
-    if (!this._editorView) return
-
-    forEachDiagnostic(this._editorView.state, function (diag) {
-      diagnostics.push(diag)
-    })
-
-    const uniqueDiagnostics = new Set<Diagnostic>()
-    diagnostics.forEach((diagnostic) => {
-      for (const knownDiagnostic of uniqueDiagnostics.values()) {
-        if (diagnosticIsEqual(diagnostic, knownDiagnostic)) {
-          return
-        }
-      }
-      uniqueDiagnostics.add(diagnostic)
-    })
-
-    this.setDiagnostics([...uniqueDiagnostics])
-  }
-
   undo() {
     if (this._editorView) {
       undo(this._editorView)

src/lang/KclSingleton.ts

@@ -217,7 +217,6 @@ export class KclManager {
       ast,
       engineCommandManager: this.engineCommandManager,
     })
-
     editorManager.addDiagnostics(await lintAst({ ast: ast }))
 
     sceneInfra.modelingSend({ type: 'code edit during sketch' })
@@ -298,7 +297,6 @@ export class KclManager {
       engineCommandManager: this.engineCommandManager,
       useFakeExecutor: true,
     })
-
     editorManager.addDiagnostics(await lintAst({ ast: ast }))
 
     this._logs = logs

src/lang/std/engineConnection.ts

@@ -7,8 +7,6 @@ import { getNodePathFromSourceRange } from 'lang/queryAst'
 import { Themes, getThemeColorForEngine, getOppositeTheme } from 'lib/theme'
 import { DefaultPlanes } from 'wasm-lib/kcl/bindings/DefaultPlanes'
 
-let lastMessage = ''
-
 // TODO(paultag): This ought to be tweakable.
 const pingIntervalMs = 10000
 
@@ -58,9 +56,6 @@ function isHighlightSetEntity_type(
 
 type WebSocketResponse = Models['WebSocketResponse_type']
 type OkWebSocketResponseData = Models['OkWebSocketResponseData_type']
-type BatchResponseMap = {
-  [key: string]: Models['BatchResponse_type']
-}
 
 type ResultCommand = CommandInfo & {
   type: 'result'
@@ -1169,6 +1164,15 @@ export enum EngineCommandManagerEvents {
  * It also maintains an {@link artifactMap} that keeps track of the state of each
  * command, and the artifacts that have been generated by those commands.
  */
+
+interface PendingMessage {
+  command: EngineCommand
+  range: SourceRange
+  idToRangeMap: { [key: string]: SourceRange }
+  resolve: (data: [Models['WebSocketResponse_type']]) => void
+  reject: (reason: string) => void
+  promise: Promise<[Models['WebSocketResponse_type']]>
+}
 export class EngineCommandManager extends EventTarget {
   /**
    * The artifactMap is a client-side representation of the commands that have been sent
@@ -1182,6 +1186,25 @@ export class EngineCommandManager extends EventTarget {
    * of the KCL code that generated it.
    */
   artifactMap: ArtifactMap = {}
+  /**
+   * The pendingCommands object is a map of the commands that have been sent to the engine that are still waiting on a reply
+   */
+  pendingCommands: {
+    [commandId: string]: PendingMessage
+  } = {}
+  /**
+   * The orderedCommands array of all the the commands sent to the engine, un-folded from batches, and made into one long
+   * list of the individual commands, this is used to process all the commands into the artifactMap
+   */
+  orderedCommands: {
+    command: EngineCommand
+    range: SourceRange
+  }[] = []
+  /**
+   * A map of the responses to the @this.orderedCommands, when processing the commands into the artifactMap, this response map allow
+   * us to look up the response by command id
+   */
+  responseMap: { [commandId: string]: OkWebSocketResponseData } = {}
   /**
    * The client-side representation of the scene command artifacts that have been sent to the server;
    * that is, the *non-modeling* commands and corresponding artifacts.
@@ -1206,7 +1229,7 @@ export class EngineCommandManager extends EventTarget {
   defaultPlanes: DefaultPlanes | null = null
   commandLogs: CommandLog[] = []
   pendingExport?: {
-    resolve: (filename?: string) => void
+    resolve: (a: null) => void
     reject: (reason: any) => void
   }
   _commandLogCallBack: (command: CommandLog[]) => void = () => {}
@@ -1435,31 +1458,92 @@ export class EngineCommandManager extends EventTarget {
           // export we send a binary blob.
           // Pass this to our export function.
           exportSave(event.data).then(() => {
-            this.pendingExport?.resolve()
+            this.pendingExport?.resolve(null)
           }, this.pendingExport?.reject)
-        } else {
-          const message: Models['WebSocketResponse_type'] = JSON.parse(
-            event.data
-          )
-          if (
+          return
+        }
+
+        const message: Models['WebSocketResponse_type'] = JSON.parse(event.data)
+        const pending = this.pendingCommands[message.request_id || '']
+
+        if (pending && !message.success) {
+          // handle bad case
+          pending.reject(`engine error: ${JSON.stringify(message.errors)}`)
+          delete this.pendingCommands[message.request_id || '']
+        }
+        if (
+          !(
+            pending &&
             message.success &&
             (message.resp.type === 'modeling' ||
-              message.resp.type === 'modeling_batch') &&
-            message.request_id
-          ) {
-            this.handleModelingCommand(
-              message.resp,
-              message.request_id,
-              message
-            )
-          } else if (
-            !message.success &&
-            message.request_id &&
-            this.artifactMap[message.request_id]
-          ) {
-            this.handleFailedModelingCommand(message.request_id, message)
-          }
+              message.resp.type === 'modeling_batch')
+          )
+        )
+          return
+
+        if (
+          message.resp.type === 'modeling' &&
+          pending.command.type === 'modeling_cmd_req' &&
+          message.request_id
+        ) {
+          this.addCommandLog({
+            type: 'receive-reliable',
+            data: message.resp,
+            id: message?.request_id || '',
+            cmd_type: pending?.command?.cmd?.type,
+          })
+
+          const modelingResponse = message.resp.data.modeling_response
+
+          Object.values(
+            this.subscriptions[modelingResponse.type] || {}
+          ).forEach((callback) => callback(modelingResponse))
+
+          this.responseMap[message.request_id] = message.resp
+        } else if (
+          message.resp.type === 'modeling_batch' &&
+          pending.command.type === 'modeling_cmd_batch_req'
+        ) {
+          let individualPendingResponses: {
+            [key: string]: Models['WebSocketRequest_type']
+          } = {}
+          pending.command.requests.forEach(({ cmd, cmd_id }) => {
+            individualPendingResponses[cmd_id] = {
+              type: 'modeling_cmd_req',
+              cmd,
+              cmd_id,
+            }
+          })
+          Object.entries(message.resp.data.responses).forEach(
+            ([key, response]) => {
+              if (!('response' in response)) return
+              const command = individualPendingResponses[key]
+              if (!command) return
+              if (command.type === 'modeling_cmd_req')
+                this.addCommandLog({
+                  type: 'receive-reliable',
+                  data: {
+                    type: 'modeling',
+                    data: {
+                      modeling_response: response.response,
+                    },
+                  },
+                  id: key,
+                  cmd_type: command?.cmd?.type,
+                })
+
+              this.responseMap[key] = {
+                type: 'modeling',
+                data: {
+                  modeling_response: response.response,
+                },
+              }
+            }
+          )
         }
+
+        pending.resolve([message])
+        delete this.pendingCommands[message.request_id || '']
       }) as EventListener)
 
       this.onEngineConnectionNewTrack = ({
@@ -1485,6 +1569,106 @@ export class EngineCommandManager extends EventTarget {
       this.onEngineConnectionStarted
     )
   }
+  handleIndividualResponse({
+    id,
+    pendingMsg,
+    response,
+  }: {
+    id: string
+    pendingMsg: {
+      command: EngineCommand
+      range: SourceRange
+    }
+    response: OkWebSocketResponseData
+  }) {
+    const command = pendingMsg
+    if (command?.command?.type !== 'modeling_cmd_req') return
+    if (response?.type !== 'modeling') return
+    const command2 = command.command.cmd
+
+    const range = command.range
+    const pathToNode = getNodePathFromSourceRange(this.getAst(), range)
+    const getParentId = (): string | undefined => {
+      if (command2.type === 'extend_path') return command2.path
+      if (command2.type === 'solid3d_get_extrusion_face_info') {
+        const edgeArtifact = this.artifactMap[command2.edge_id]
+        // edges's parent id is to the original "start_path" artifact
+        if (edgeArtifact && edgeArtifact.parentId) {
+          return edgeArtifact.parentId
+        }
+      }
+      if (command2.type === 'close_path') return command2.path_id
+      if (command2.type === 'extrude') return command2.target
+      // handle other commands that have a parent here
+    }
+    const modelingResponse = response.data.modeling_response
+
+    if (command) {
+      const parentId = getParentId()
+      const artifact = {
+        type: 'result',
+        range: range,
+        pathToNode,
+        commandType: command.command.cmd.type,
+        parentId: parentId,
+      } as ArtifactMapCommand & { extrusions?: string[] }
+      this.artifactMap[id] = artifact
+      if (command2.type === 'extrude') {
+        ;(artifact as any).target = command2.target
+        if (this.artifactMap[command2.target]?.commandType === 'start_path') {
+          if ((this.artifactMap[command2.target] as any)?.extrusions?.length) {
+            ;(this.artifactMap[command2.target] as any).extrusions.push(id)
+          } else {
+            ;(this.artifactMap[command2.target] as any).extrusions = [id]
+          }
+        }
+      }
+      this.artifactMap[id] = artifact
+      if (
+        (command2.type === 'entity_linear_pattern' &&
+          modelingResponse.type === 'entity_linear_pattern') ||
+        (command2.type === 'entity_circular_pattern' &&
+          modelingResponse.type === 'entity_circular_pattern')
+      ) {
+        const entities = modelingResponse.data.entity_ids
+        entities?.forEach((entity: string) => {
+          this.artifactMap[entity] = artifact
+        })
+      }
+      if (
+        command2.type === 'solid3d_get_extrusion_face_info' &&
+        modelingResponse.type === 'solid3d_get_extrusion_face_info'
+      ) {
+        const parent = this.artifactMap[parentId || '']
+        modelingResponse.data.faces.forEach((face) => {
+          if (face.cap !== 'none' && face.face_id && parent) {
+            this.artifactMap[face.face_id] = {
+              ...parent,
+              commandType: 'solid3d_get_extrusion_face_info',
+              additionalData: {
+                type: 'cap',
+                info: face.cap === 'bottom' ? 'start' : 'end',
+              },
+            }
+          }
+          const curveArtifact = this.artifactMap[face?.curve_id || '']
+          if (curveArtifact && face?.face_id) {
+            this.artifactMap[face.face_id] = {
+              ...curveArtifact,
+              commandType: 'solid3d_get_extrusion_face_info',
+            }
+          }
+        })
+      }
+    } else if (command) {
+      this.artifactMap[id] = {
+        type: 'result',
+        commandType: command2.type,
+        range,
+        pathToNode,
+      } as ArtifactMapCommand & { extrusions?: string[] }
+    }
+  }
 
   handleResize({
     streamWidth,
@@ -1509,233 +1693,7 @@ export class EngineCommandManager extends EventTarget {
     }
     this.engineConnection?.send(resizeCmd)
   }
-  handleModelingCommand(
-    message: OkWebSocketResponseData,
-    id: string,
-    raw: WebSocketResponse
-  ) {
-    if (!(message.type === 'modeling' || message.type === 'modeling_batch')) {
-      return
-    }
 
-    const command = this.artifactMap[id]
-    let modelingResponse: Models['OkModelingCmdResponse_type'] = {
-      type: 'empty',
-    }
-    if ('modeling_response' in message.data) {
-      modelingResponse = message.data.modeling_response
-    }
-    if (
-      command?.type === 'pending' &&
-      command.commandType === 'batch' &&
-      command?.additionalData?.type === 'batch-ids'
-    ) {
-      if ('responses' in message.data) {
-        const batchResponse = message.data.responses as BatchResponseMap
-        // Iterate over the map of responses.
-        Object.entries(batchResponse).forEach(([key, response]) => {
-          // If the response is a success, we resolve the promise.
-          if ('response' in response && response.response) {
-            this.handleModelingCommand(
-              {
-                type: 'modeling',
-                data: {
-                  modeling_response: response.response,
-                },
-              },
-              key,
-              {
-                request_id: key,
-                resp: {
-                  type: 'modeling',
-                  data: {
-                    modeling_response: response.response,
-                  },
-                },
-                success: true,
-              }
-            )
-          } else if ('errors' in response) {
-            this.handleFailedModelingCommand(key, {
-              request_id: key,
-              success: false,
-              errors: response.errors,
-            })
-          }
-        })
-      } else {
-        command.additionalData.ids.forEach((id) => {
-          this.handleModelingCommand(message, id, raw)
-        })
-      }
-      // batch artifact is just a container, we don't need to keep it
-      // once we process all the commands inside it
-      const resolve = command.resolve
-      delete this.artifactMap[id]
-      resolve({
-        id,
-        commandType: command.commandType,
-        range: command.range,
-        raw,
-      })
-      return
-    }
-    const sceneCommand = this.sceneCommandArtifacts[id]
-    this.addCommandLog({
-      type: 'receive-reliable',
-      data: message,
-      id,
-      cmd_type: command?.commandType || sceneCommand?.commandType,
-    })
-    Object.values(this.subscriptions[modelingResponse.type] || {}).forEach(
-      (callback) => callback(modelingResponse)
-    )
-
-    if (command && command.type === 'pending') {
-      const resolve = command.resolve
-      const oldArtifact = this.artifactMap[id] as ArtifactMapCommand & {
-        extrusions?: string[]
-      }
-      const artifact = {
-        type: 'result',
-        range: command.range,
-        pathToNode: command.pathToNode,
-        commandType: command.commandType,
-        parentId: command.parentId ? command.parentId : undefined,
-        data: modelingResponse,
-        raw,
-      } as ArtifactMapCommand & { extrusions?: string[] }
-      if (oldArtifact?.extrusions) {
-        artifact.extrusions = oldArtifact.extrusions
-      }
-      this.artifactMap[id] = artifact
-      if (
-        (command.commandType === 'entity_linear_pattern' &&
-          modelingResponse.type === 'entity_linear_pattern') ||
-        (command.commandType === 'entity_circular_pattern' &&
-          modelingResponse.type === 'entity_circular_pattern')
-      ) {
-        const entities = modelingResponse.data.entity_ids
-        entities?.forEach((entity: string) => {
-          this.artifactMap[entity] = artifact
-        })
-      }
-      if (
-        command?.commandType === 'solid3d_get_extrusion_face_info' &&
-        modelingResponse.type === 'solid3d_get_extrusion_face_info'
-      ) {
-        const parent = this.artifactMap[command?.parentId || '']
-        modelingResponse.data.faces.forEach((face) => {
-          if (face.cap !== 'none' && face.face_id && parent) {
-            this.artifactMap[face.face_id] = {
-              ...parent,
-              commandType: 'solid3d_get_extrusion_face_info',
-              additionalData: {
-                type: 'cap',
-                info: face.cap === 'bottom' ? 'start' : 'end',
-              },
-            }
-          }
-          const curveArtifact = this.artifactMap[face?.curve_id || '']
-          if (curveArtifact && face?.face_id) {
-            this.artifactMap[face.face_id] = {
-              ...curveArtifact,
-              commandType: 'solid3d_get_extrusion_face_info',
-            }
-          }
-        })
-      }
-      resolve({
-        id,
-        commandType: command.commandType,
-        range: command.range,
-        data: modelingResponse,
-        raw,
-      })
-    } else if (sceneCommand && sceneCommand.type === 'pending') {
-      const resolve = sceneCommand.resolve
-      const artifact = {
-        type: 'result',
-        range: sceneCommand.range,
-        pathToNode: sceneCommand.pathToNode,
-        commandType: sceneCommand.commandType,
-        parentId: sceneCommand.parentId ? sceneCommand.parentId : undefined,
-        data: modelingResponse,
-        raw,
-      } as const
-      this.sceneCommandArtifacts[id] = artifact
-      resolve({
-        id,
-        commandType: sceneCommand.commandType,
-        range: sceneCommand.range,
-        data: modelingResponse,
-        raw,
-      })
-    } else if (command) {
-      this.artifactMap[id] = {
-        type: 'result',
-        commandType: command?.commandType,
-        range: command?.range,
-        pathToNode: command?.pathToNode,
-        data: modelingResponse,
-        raw,
-      }
-    } else {
-      this.sceneCommandArtifacts[id] = {
-        type: 'result',
-        commandType: sceneCommand?.commandType,
-        range: sceneCommand?.range,
-        pathToNode: sceneCommand?.pathToNode,
-        data: modelingResponse,
-        raw,
-      }
-    }
-  }
-  handleFailedModelingCommand(id: string, raw: WebSocketResponse) {
-    const failed = raw as Models['FailureWebSocketResponse_type']
-    const errors = failed.errors
-    if (!id) return
-    const command = this.artifactMap[id]
-    if (command && command.type === 'pending') {
-      this.artifactMap[id] = {
-        type: 'failed',
-        range: command.range,
-        pathToNode: command.pathToNode,
-        commandType: command.commandType,
-        parentId: command.parentId ? command.parentId : undefined,
-        errors,
-      }
-      if (
-        command?.type === 'pending' &&
-        command.commandType === 'batch' &&
-        command?.additionalData?.type === 'batch-ids'
-      ) {
-        command.additionalData.ids.forEach((id) => {
-          this.handleFailedModelingCommand(id, raw)
-        })
-      }
-      // batch artifact is just a container, we don't need to keep it
-      // once we process all the commands inside it
-      const resolve = command.resolve
-      delete this.artifactMap[id]
-      resolve({
-        id,
-        commandType: command.commandType,
-        range: command.range,
-        errors,
-        raw,
-      })
-    } else {
-      this.artifactMap[id] = {
-        type: 'failed',
-        range: command.range,
-        pathToNode: command.pathToNode,
-        commandType: command.commandType,
-        parentId: command.parentId ? command.parentId : undefined,
-        errors,
-      }
-    }
-  }
   tearDown(opts?: { idleMode: boolean }) {
     if (this.engineConnection) {
       this.engineConnection.removeEventListener(
@@ -1768,6 +1726,8 @@ export class EngineCommandManager extends EventTarget {
   }
   async startNewSession() {
     this.artifactMap = {}
+    this.orderedCommands = []
+    this.responseMap = {}
     await this.initPlanes()
   }
   subscribeTo<T extends ModelTypes>({
@@ -1841,13 +1801,13 @@ export class EngineCommandManager extends EventTarget {
   sendSceneCommand(
     command: EngineCommand,
     forceWebsocket = false
-  ): Promise<any> {
+  ): Promise<Models['WebSocketResponse_type'] | null> {
     if (this.engineConnection === undefined) {
-      return Promise.resolve()
+      return Promise.resolve(null)
     }
 
     if (!this.engineConnection?.isReady()) {
-      return Promise.resolve()
+      return Promise.resolve(null)
     }
 
     if (
@@ -1866,19 +1826,13 @@ export class EngineCommandManager extends EventTarget {
       })
     }
 
-    if (
-      command.type === 'modeling_cmd_req' &&
-      command.cmd.type !== lastMessage
-    ) {
-      lastMessage = command.cmd.type
-    }
     if (command.type === 'modeling_cmd_batch_req') {
       this.engineConnection?.send(command)
       // TODO - handlePendingCommands does not handle batch commands
       // return this.handlePendingCommand(command.requests[0].cmd_id, command.cmd)
-      return Promise.resolve()
+      return Promise.resolve(null)
     }
-    if (command.type !== 'modeling_cmd_req') return Promise.resolve()
+    if (command.type !== 'modeling_cmd_req') return Promise.resolve(null)
     const cmd = command.cmd
     if (
       (cmd.type === 'camera_drag_move' ||
@@ -1891,7 +1845,7 @@ export class EngineCommandManager extends EventTarget {
       ;(cmd as any).sequence = this.outSequence
       this.outSequence++
       this.engineConnection?.unreliableSend(command)
-      return Promise.resolve()
+      return Promise.resolve(null)
     } else if (
       cmd.type === 'highlight_set_entity' &&
       this.engineConnection?.unreliableDataChannel
@@ -1899,7 +1853,7 @@ export class EngineCommandManager extends EventTarget {
       cmd.sequence = this.outSequence
       this.outSequence++
       this.engineConnection?.unreliableSend(command)
-      return Promise.resolve()
+      return Promise.resolve(null)
     } else if (
       cmd.type === 'mouse_move' &&
       this.engineConnection.unreliableDataChannel
@@ -1907,9 +1861,9 @@ export class EngineCommandManager extends EventTarget {
       cmd.sequence = this.outSequence
       this.outSequence++
       this.engineConnection?.unreliableSend(command)
-      return Promise.resolve()
+      return Promise.resolve(null)
     } else if (cmd.type === 'export') {
-      const promise = new Promise((resolve, reject) => {
+      const promise = new Promise<null>((resolve, reject) => {
         this.pendingExport = { resolve, reject }
       })
       this.engineConnection?.send(command)
@@ -1922,194 +1876,15 @@ export class EngineCommandManager extends EventTarget {
       ;(cmd as any).sequence = this.outSequence++
     }
     // since it's not mouse drag or highlighting send over TCP and keep track of the command
-    this.engineConnection?.send(command)
-    return this.handlePendingSceneCommand(command.cmd_id, command.cmd)
-  }
-  sendModelingCommand({
-    id,
-    range,
-    command,
-    ast,
-    idToRangeMap,
-  }: {
-    id: string
-    range: SourceRange
-    command: EngineCommand
-    ast: Program
-    idToRangeMap?: { [key: string]: SourceRange }
-  }): Promise<ResolveCommand | void> {
-    if (this.engineConnection === undefined) {
-      return Promise.resolve()
-    }
-
-    if (!this.engineConnection?.isReady()) {
-      return Promise.resolve()
-    }
-    if (typeof command !== 'string') {
-      this.addCommandLog({
-        type: 'send-modeling',
-        data: command,
-      })
-    } else {
-      this.addCommandLog({
-        type: 'send-modeling',
-        data: JSON.parse(command),
-      })
-    }
-    this.engineConnection?.send(command)
-    if (typeof command !== 'string' && command.type === 'modeling_cmd_req') {
-      return this.handlePendingCommand(id, command?.cmd, ast, range)
-    } else if (
-      typeof command !== 'string' &&
-      command.type === 'modeling_cmd_batch_req'
-    ) {
-      return this.handlePendingBatchCommand(id, command.requests, idToRangeMap)
-    } else if (typeof command === 'string') {
-      const parseCommand: EngineCommand = JSON.parse(command)
-      if (parseCommand.type === 'modeling_cmd_req') {
-        return this.handlePendingCommand(id, parseCommand?.cmd, ast, range)
-      } else if (parseCommand.type === 'modeling_cmd_batch_req') {
-        return this.handlePendingBatchCommand(
-          id,
-          parseCommand.requests,
-          idToRangeMap
-        )
-      }
-    }
-    return Promise.reject(new Error('Expected unreachable reached'))
-  }
-  handlePendingSceneCommand(
-    id: string,
-    command: Models['ModelingCmd_type'],
-    ast?: Program,
-    range?: SourceRange
-  ) {
-    let resolve: (val: any) => void = () => {}
-    const promise = new Promise((_resolve, reject) => {
-      resolve = _resolve
-    })
-    const pathToNode = ast
-      ? getNodePathFromSourceRange(ast, range || [0, 0])
-      : []
-    this.sceneCommandArtifacts[id] = {
-      range: range || [0, 0],
-      pathToNode,
-      type: 'pending',
-      commandType: command.type,
-      promise,
-      resolve,
-    }
-    return promise
-  }
-  handlePendingCommand(
-    id: string,
-    command: Models['ModelingCmd_type'],
-    ast?: Program,
-    range?: SourceRange
-  ): Promise<ResolveCommand | void> {
-    let resolve: (val: any) => void = () => {}
-    const promise: Promise<ResolveCommand | void> = new Promise(
-      (_resolve, reject) => {
-        resolve = _resolve
-      }
-    )
-    const getParentId = (): string | undefined => {
-      if (command.type === 'extend_path') return command.path
-      if (command.type === 'solid3d_get_extrusion_face_info') {
-        const edgeArtifact = this.artifactMap[command.edge_id]
-        // edges's parent id is to the original "start_path" artifact
-        if (edgeArtifact && edgeArtifact.parentId) {
-          return edgeArtifact.parentId
-        }
-      }
-      if (command.type === 'close_path') return command.path_id
-      if (command.type === 'extrude') return command.target
-      // handle other commands that have a parent here
-    }
-    const pathToNode = ast
-      ? getNodePathFromSourceRange(ast, range || [0, 0])
-      : []
-    this.artifactMap[id] = {
-      range: range || [0, 0],
-      pathToNode,
-      type: 'pending',
-      commandType: command.type,
-      parentId: getParentId(),
-      promise,
-      resolve,
-    }
-    if (command.type === 'extrude') {
-      this.artifactMap[id] = {
-        range: range || [0, 0],
-        pathToNode,
-        type: 'pending',
-        commandType: 'extrude',
-        parentId: getParentId(),
-        promise,
-        target: command.target,
-        resolve,
-      }
-      const target = this.artifactMap[command.target]
-      if (target.commandType === 'start_path') {
-        // tsc cannot infer that target can have extrusions
-        // from the commandType (why?) so we need to cast it
-        const typedTarget = target as (
-          | PendingCommand
-          | ResultCommand
-          | FailedCommand
-        ) & { extrusions?: string[] }
-        if (typedTarget?.extrusions?.length) {
-          typedTarget.extrusions.push(id)
-        } else {
-          typedTarget.extrusions = [id]
-        }
-        // Update in the map.
-        this.artifactMap[command.target] = typedTarget
-      }
-    }
-    return promise
-  }
-  async handlePendingBatchCommand(
-    id: string,
-    commands: Models['ModelingCmdReq_type'][],
-    idToRangeMap?: { [key: string]: SourceRange },
-    ast?: Program,
-    range?: SourceRange
-  ): Promise<ResolveCommand | void> {
-    let resolve: (val: any) => void = () => {}
-    const promise: Promise<ResolveCommand | void> = new Promise(
-      (_resolve, reject) => {
-        resolve = _resolve
-      }
-    )
-
-    if (!idToRangeMap) {
-      return Promise.reject(
-        new Error('idToRangeMap is required for batch commands')
-      )
-    }
-
-    // Add the overall batch command to the artifact map just so we can track all of the
-    // individual commands that are part of the batch.
-    // we'll delete this artifact once all of the individual commands have been processed.
-    this.artifactMap[id] = {
-      range: range || [0, 0],
-      pathToNode: [],
-      type: 'pending',
-      commandType: 'batch',
-      additionalData: { type: 'batch-ids', ids: commands.map((c) => c.cmd_id) },
-      parentId: undefined,
-      promise,
-      resolve,
-    }
-
-    Promise.all(
-      commands.map((c) =>
-        this.handlePendingCommand(c.cmd_id, c.cmd, ast, idToRangeMap[c.cmd_id])
-      )
-    )
-    return promise
+    return this.sendCommand(command.cmd_id, {
+      command,
+      idToRangeMap: {},
+      range: [0, 0],
+    }).then(([a]) => a)
   }
+  /**
+   * A wrapper around the sendCommand where all inputs are JSON strings
+   */
   async sendModelingCommandFromWasm(
     id: string,
     rangeStr: string,
@@ -2132,53 +1907,88 @@ export class EngineCommandManager extends EventTarget {
       return Promise.reject(new Error('commandStr is undefined'))
     }
     const range: SourceRange = JSON.parse(rangeStr)
+    const command: EngineCommand = JSON.parse(commandStr)
     const idToRangeMap: { [key: string]: SourceRange } =
       JSON.parse(idToRangeStr)
 
-    const command: EngineCommand = JSON.parse(commandStr)
-
-    // We only care about the modeling command response.
-    return this.sendModelingCommand({
-      id,
-      range,
+    const resp = await this.sendCommand(id, {
       command,
-      ast: this.getAst(),
+      range,
       idToRangeMap,
-    }).then((resp) => {
-      if (!resp) {
-        return Promise.reject(
-          new Error(
-            'returning modeling cmd response to the rust side is undefined or null'
-          )
-        )
-      }
-      return JSON.stringify(resp.raw)
     })
+    return JSON.stringify(resp[0])
   }
-  async commandResult(id: string): Promise<any> {
-    const command = this.artifactMap[id]
-    if (!command) {
-      return Promise.reject(new Error('No command found'))
+  /**
+   * Common send command function used for both modeling and scene commands
+   * So that both have a common way to send pending commands with promises for the responses
+   */
+  async sendCommand(
+    id: string,
+    message: {
+      command: PendingMessage['command']
+      range: PendingMessage['range']
+      idToRangeMap: PendingMessage['idToRangeMap']
     }
-    if (command.type === 'result') {
-      return command.data
-    } else if (command.type === 'failed') {
-      return Promise.resolve(command.errors)
+  ): Promise<[Models['WebSocketResponse_type']]> {
+    const { promise, resolve, reject } = promiseFactory<any>()
+    this.pendingCommands[id] = {
+      resolve,
+      reject,
+      promise,
+      command: message.command,
+      range: message.range,
+      idToRangeMap: message.idToRangeMap,
     }
-    return command.promise
+    if (message.command.type === 'modeling_cmd_req') {
+      this.orderedCommands.push({
+        command: message.command,
+        range: message.range,
+      })
+    } else if (message.command.type === 'modeling_cmd_batch_req') {
+      message.command.requests.forEach((req) => {
+        const cmd: EngineCommand = {
+          type: 'modeling_cmd_req',
+          cmd_id: req.cmd_id,
+          cmd: req.cmd,
+        }
+        this.orderedCommands.push({
+          command: cmd,
+          range: message.idToRangeMap[req.cmd_id || ''],
+        })
+      })
+    }
+    this.engineConnection?.send(message.command)
+    return promise
   }
-  async waitForAllCommands(): Promise<{
-    artifactMap: ArtifactMap
-  }> {
+  /**
+   * When an execution takes place we want to wait until we've got replies for all of the commands
+   * When this is done when we build the artifact map synchronously.
+   */
+  async waitForAllCommands() {
     const pendingCommands = Object.values(this.artifactMap).filter(
       ({ type }) => type === 'pending'
     ) as PendingCommand[]
     const proms = pendingCommands.map(({ promise }) => promise)
-    await Promise.all(proms)
 
-    return {
-      artifactMap: this.artifactMap,
-    }
+    const otherPending = Object.values(this.pendingCommands).map(
+      (a) => a.promise
+    )
+    await Promise.all([...proms, otherPending])
+    this.orderedCommands.forEach(({ command, range }) => {
+      // expect all to be `modeling_cmd_req` as batch commands have
+      // already been expanded before being added to orderedCommands
+      if (command.type !== 'modeling_cmd_req') return
+      const id = command.cmd_id
+      const response = this.responseMap[id]
+      this.handleIndividualResponse({
+        id,
+        pendingMsg: {
+          command,
+          range,
+        },
+        response,
+      })
+    })
   }
   private async initPlanes() {
     if (this.planesInitialized()) return
@@ -2209,7 +2019,7 @@ export class EngineCommandManager extends EventTarget {
     this.onPlaneSelectCallback = callback
   }
 
-  async setPlaneHidden(id: string, hidden: boolean): Promise<string> {
+  async setPlaneHidden(id: string, hidden: boolean) {
     return await this.sendSceneCommand({
       type: 'modeling_cmd_req',
       cmd_id: uuidv4(),
@@ -2250,3 +2060,13 @@ export class EngineCommandManager extends EventTarget {
     return undefined
   }
 }
+
+function promiseFactory<T>() {
+  let resolve: (value: T | PromiseLike<T>) => void = () => {}
+  let reject: (value: T | PromiseLike<T>) => void = () => {}
+  const promise = new Promise<T>((_resolve, _reject) => {
+    resolve = _resolve
+    reject = _reject
+  })
+  return { promise, resolve, reject }
+}

src/lib/selections.ts

@@ -87,16 +87,20 @@ export async function getEventForSelectWithPoint(
     // there's plans to get the faceId back from the solid2d creation
     // https://github.com/KittyCAD/engine/issues/2094
     // at which point we can add it to the artifact map and remove this logic
-    const parentId = (
-      await engineCommandManager.sendSceneCommand({
-        type: 'modeling_cmd_req',
-        cmd: {
-          type: 'entity_get_parent_id',
-          entity_id: data.entity_id,
-        },
-        cmd_id: uuidv4(),
-      })
-    )?.data?.data?.entity_id
+    const resp = await engineCommandManager.sendSceneCommand({
+      type: 'modeling_cmd_req',
+      cmd: {
+        type: 'entity_get_parent_id',
+        entity_id: data.entity_id,
+      },
+      cmd_id: uuidv4(),
+    })
+    const parentId =
+      resp?.success &&
+      resp?.resp?.type === 'modeling' &&
+      resp?.resp?.data?.modeling_response?.type === 'entity_get_parent_id'
+        ? resp?.resp?.data?.modeling_response?.data?.entity_id
+        : ''
     const parentArtifact = engineCommandManager.artifactMap[parentId]
     if (parentArtifact) {
       _artifact = parentArtifact
@@ -576,18 +580,22 @@ export async function sendSelectEventToEngine(
     el,
     ...streamDimensions,
   })
-  const result: Models['SelectWithPoint_type'] = await engineCommandManager
-    .sendSceneCommand({
-      type: 'modeling_cmd_req',
-      cmd: {
-        type: 'select_with_point',
-        selected_at_window: { x, y },
-        selection_type: 'add',
-      },
-      cmd_id: uuidv4(),
-    })
-    .then((res) => res.data.data)
-  return result
+  const res = await engineCommandManager.sendSceneCommand({
+    type: 'modeling_cmd_req',
+    cmd: {
+      type: 'select_with_point',
+      selected_at_window: { x, y },
+      selection_type: 'add',
+    },
+    cmd_id: uuidv4(),
+  })
+  if (
+    res?.success &&
+    res?.resp?.type === 'modeling' &&
+    res?.resp?.data?.modeling_response.type === 'select_with_point'
+  )
+    return res?.resp?.data?.modeling_response?.data
+  return { entity_id: '' }
 }
 
 export function updateSelections(

src/wasm-lib/kcl/src/ast/types.rs

@@ -2880,30 +2880,6 @@ impl BinaryExpression {
         pipe_info: &PipeInfo,
         ctx: &ExecutorContext,
     ) -> Result<MemoryItem, KclError> {
-        // First check if we are doing short-circuiting logical operator.
-        if self.operator == BinaryOperator::LogicalOr {
-            let left_json_value = self.left.get_result(memory, pipe_info, ctx).await?.get_json_value()?;
-            let left = json_to_bool(&left_json_value);
-            if left {
-                // Short-circuit.
-                return Ok(MemoryItem::UserVal(UserVal {
-                    value: serde_json::Value::Bool(left),
-                    meta: vec![Metadata {
-                        source_range: self.into(),
-                    }],
-                }));
-            }
-
-            let right_json_value = self.right.get_result(memory, pipe_info, ctx).await?.get_json_value()?;
-            let right = json_to_bool(&right_json_value);
-            return Ok(MemoryItem::UserVal(UserVal {
-                value: serde_json::Value::Bool(right),
-                meta: vec![Metadata {
-                    source_range: self.into(),
-                }],
-            }));
-        }
-
         let left_json_value = self.left.get_result(memory, pipe_info, ctx).await?.get_json_value()?;
         let right_json_value = self.right.get_result(memory, pipe_info, ctx).await?.get_json_value()?;
 
@@ -2933,9 +2909,6 @@ impl BinaryExpression {
             BinaryOperator::Div => (left / right).into(),
             BinaryOperator::Mod => (left % right).into(),
             BinaryOperator::Pow => (left.powf(right)).into(),
-            BinaryOperator::LogicalOr => {
-                unreachable!("LogicalOr should have been handled above")
-            }
         };
 
         Ok(MemoryItem::UserVal(UserVal {
@@ -2977,27 +2950,6 @@ pub fn parse_json_value_as_string(j: &serde_json::Value) -> Option<String> {
     }
 }
 
-pub fn json_to_bool(j: &serde_json::Value) -> bool {
-    match j {
-        JValue::Null => false,
-        JValue::Bool(b) => *b,
-        JValue::Number(n) => {
-            if let Some(n) = n.as_u64() {
-                n != 0
-            } else if let Some(n) = n.as_i64() {
-                n != 0
-            } else if let Some(x) = n.as_f64() {
-                x != 0.0 && !x.is_nan()
-            } else {
-                false
-            }
-        }
-        JValue::String(s) => !s.is_empty(),
-        JValue::Array(a) => !a.is_empty(),
-        JValue::Object(_) => false,
-    }
-}
-
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, FromStr, Display, Bake)]
 #[databake(path = kcl_lib::ast::types)]
 #[ts(export)]
@@ -3028,10 +2980,6 @@ pub enum BinaryOperator {
     #[serde(rename = "^")]
     #[display("^")]
     Pow,
-    /// Logical OR.
-    #[serde(rename = "||")]
-    #[display("||")]
-    LogicalOr,
 }
 
 /// Mathematical associativity.
@@ -3060,7 +3008,6 @@ impl BinaryOperator {
             BinaryOperator::Div => *b"div",
             BinaryOperator::Mod => *b"mod",
             BinaryOperator::Pow => *b"pow",
-            BinaryOperator::LogicalOr => *b"lor",
         }
     }
 
@@ -3071,7 +3018,6 @@ impl BinaryOperator {
             BinaryOperator::Add | BinaryOperator::Sub => 11,
             BinaryOperator::Mul | BinaryOperator::Div | BinaryOperator::Mod => 12,
             BinaryOperator::Pow => 6,
-            BinaryOperator::LogicalOr => 3,
         }
     }
 
@@ -3079,7 +3025,7 @@ impl BinaryOperator {
     /// Taken from <https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_precedence#table>
     pub fn associativity(&self) -> Associativity {
         match self {
-            Self::Add | Self::Sub | Self::Mul | Self::Div | Self::Mod | Self::LogicalOr => Associativity::Left,
+            Self::Add | Self::Sub | Self::Mul | Self::Div | Self::Mod => Associativity::Left,
             Self::Pow => Associativity::Right,
         }
     }
@@ -3143,21 +3089,6 @@ impl UnaryExpression {
         pipe_info: &PipeInfo,
         ctx: &ExecutorContext,
     ) -> Result<MemoryItem, KclError> {
-        if self.operator == UnaryOperator::Not {
-            let value = self
-                .argument
-                .get_result(memory, pipe_info, ctx)
-                .await?
-                .get_json_value()?;
-            let negated = !json_to_bool(&value);
-            return Ok(MemoryItem::UserVal(UserVal {
-                value: serde_json::Value::Bool(negated),
-                meta: vec![Metadata {
-                    source_range: self.into(),
-                }],
-            }));
-        }
-
         let num = parse_json_number_as_f64(
             &self
                 .argument

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

@@ -2513,57 +2513,6 @@ let shape = layer() |> patternTransform(10, transform, %)"#;
         );
     }
 
-    #[tokio::test(flavor = "multi_thread")]
-    async fn test_execute_ycombinator_is_even() {
-        let ast = r#"
-// Heavily inspired by: https://raganwald.com/2018/09/10/why-y.html
-fn why = (f) => {
-  fn inner = (maker) => {
-    fn inner2 = (x) => {
-      return f(maker(maker), x)
-    }
-    return inner2
-  }
-
-  return inner(
-    (maker) => {
-      fn inner2 = (x) => {
-        return f(maker(maker), x)
-      }
-      return inner2
-    }
-  )
-}
-
-fn innerIsEven = (self, n) => {
-  return !n || !self(n - 1)
-}
-
-const isEven = why(innerIsEven)
-
-const two = isEven(2)
-const three = isEven(3)
-"#;
-
-        let memory = parse_execute(ast).await.unwrap();
-        assert_eq!(
-            serde_json::json!(true),
-            memory
-                .get("two", SourceRange::default())
-                .unwrap()
-                .get_json_value()
-                .unwrap()
-        );
-        assert_eq!(
-            serde_json::json!(false),
-            memory
-                .get("three", SourceRange::default())
-                .unwrap()
-                .get_json_value()
-                .unwrap()
-        );
-    }
-
     #[tokio::test(flavor = "multi_thread")]
     async fn test_math_execute_with_functions() {
         let ast = r#"const myVar = 2 + min(100, -1 + legLen(5, 3))"#;

src/wasm-lib/kcl/src/parser/parser_impl.rs

@@ -299,7 +299,6 @@ fn binary_operator(i: TokenSlice) -> PResult<BinaryOperator> {
             "*" => BinaryOperator::Mul,
             "%" => BinaryOperator::Mod,
             "^" => BinaryOperator::Pow,
-            "||" => BinaryOperator::LogicalOr,
             _ => {
                 return Err(KclError::Syntax(KclErrorDetails {
                     source_ranges: token.as_source_ranges(),
@@ -1137,11 +1136,11 @@ fn unary_expression(i: TokenSlice) -> PResult<UnaryExpression> {
     let (operator, op_token) = any
         .try_map(|token: Token| match token.token_type {
             TokenType::Operator if token.value == "-" => Ok((UnaryOperator::Neg, token)),
+            // TODO: negation. Original parser doesn't support `not` yet.
             TokenType::Operator => Err(KclError::Syntax(KclErrorDetails {
                 source_ranges: token.as_source_ranges(),
                 message: format!("{EXPECTED} but found {} which is an operator, but not a unary one (unary operators apply to just a single operand, your operator applies to two or more operands)", token.value.as_str(),),
             })),
-            TokenType::Bang => Ok((UnaryOperator::Not, token)),
             other => Err(KclError::Syntax(KclErrorDetails { source_ranges: token.as_source_ranges(), message: format!("{EXPECTED} but found {} which is {}", token.value.as_str(), other,) })),
         })
         .context(expected("a unary expression, e.g. -x or -3"))

src/wasm-lib/kcl/src/parser/parser_impl/error.rs

@@ -79,7 +79,7 @@ impl From<ParseError<&[Token], ContextError>> for KclError {
         // See https://github.com/KittyCAD/modeling-app/issues/784
         KclError::Syntax(KclErrorDetails {
             source_ranges: bad_token.as_source_ranges(),
-            message: format!("Unexpected token: {}", bad_token.value),
+            message: "Unexpected token".to_string(),
         })
     }
 }

src/wasm-lib/kcl/src/token/tokeniser.rs

@@ -90,7 +90,7 @@ fn word(i: &mut Located<&str>) -> PResult<Token> {
 
 fn operator(i: &mut Located<&str>) -> PResult<Token> {
     let (value, range) = alt((
-        ">=", "<=", "==", "=>", "!= ", "|>", "*", "+", "-", "/", "%", "=", "<", ">", r"\", "||", "|", "^",
+        ">=", "<=", "==", "=>", "!= ", "|>", "*", "+", "-", "/", "%", "=", "<", ">", r"\", "|", "^",
     ))
     .with_span()
     .parse_next(i)?;