path to node progress

Refactor: Move error types into their own submodule

package.json

@@ -136,7 +136,7 @@
     "prettier": "^2.8.0",
     "setimmediate": "^1.0.5",
     "tailwindcss": "^3.3.6",
-    "vite": "^4.5.1",
+    "vite": "^4.5.2",
     "vite-plugin-eslint": "^1.8.1",
     "vite-tsconfig-paths": "^4.2.1",
     "wait-on": "^7.2.0",

src-tauri/Cargo.lock

@@ -1242,9 +1242,9 @@ dependencies = [
 
 [[package]]
 name = "h2"
-version = "0.3.20"
+version = "0.3.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "97ec8491ebaf99c8eaa73058b045fe58073cd6be7f596ac993ced0b0a0c01049"
+checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9"
 dependencies = [
  "bytes",
  "fnv",
@@ -1252,7 +1252,7 @@ dependencies = [
  "futures-sink",
  "futures-util",
  "http",
- "indexmap 1.9.3",
+ "indexmap 2.0.0",
  "slab",
  "tokio",
  "tokio-util",

src/lang/std/engineConnection.ts

@@ -48,6 +48,72 @@ type Timeout = ReturnType<typeof setTimeout>
 
 type ClientMetrics = Models['ClientMetrics_type']
 
+type Value<T, U> = U extends undefined
+  ? { type: T; value: U }
+  : U extends void
+  ? { type: T }
+  : { type: T; value: U }
+
+type State<T, U> = Value<T, U>
+
+enum EngineConnectionStateType {
+  Fresh = 'fresh',
+  Connecting = 'connecting',
+  ConnectionEstablished = 'connection-established',
+  Disconnected = 'disconnected',
+}
+
+enum DisconnectedType {
+  Error = 'error',
+  Timeout = 'timeout',
+  Quit = 'quit',
+}
+
+type DisconnectedValue =
+  | State<DisconnectedType.Error, Error | undefined>
+  | State<DisconnectedType.Timeout, void>
+  | State<DisconnectedType.Quit, void>
+
+// These are ordered by the expected sequence.
+enum ConnectingType {
+  WebSocketConnecting = 'websocket-connecting',
+  WebSocketEstablished = 'websocket-established',
+  PeerConnectionCreated = 'peer-connection-created',
+  ICEServersSet = 'ice-servers-set',
+  SetLocalDescription = 'set-local-description',
+  OfferedSdp = 'offered-sdp',
+  ReceivedSdp = 'received-sdp',
+  SetRemoteDescription = 'set-remote-description',
+  WebRTCConnecting = 'webrtc-connecting',
+  ICECandidateReceived = 'ice-candidate-received',
+  TrackReceived = 'track-received',
+  DataChannelRequested = 'data-channel-requested',
+  DataChannelConnecting = 'data-channel-connecting',
+  DataChannelEstablished = 'data-channel-established',
+}
+
+type ConnectingValue =
+  | State<ConnectingType.WebSocketConnecting, void>
+  | State<ConnectingType.WebSocketEstablished, void>
+  | State<ConnectingType.PeerConnectionCreated, void>
+  | State<ConnectingType.ICEServersSet, void>
+  | State<ConnectingType.SetLocalDescription, void>
+  | State<ConnectingType.OfferedSdp, void>
+  | State<ConnectingType.ReceivedSdp, void>
+  | State<ConnectingType.SetRemoteDescription, void>
+  | State<ConnectingType.WebRTCConnecting, void>
+  | State<ConnectingType.TrackReceived, void>
+  | State<ConnectingType.ICECandidateReceived, void>
+  | State<ConnectingType.DataChannelRequested, string>
+  | State<ConnectingType.DataChannelConnecting, string>
+  | State<ConnectingType.DataChannelEstablished, void>
+
+type EngineConnectionState =
+  | State<EngineConnectionStateType.Fresh, void>
+  | State<EngineConnectionStateType.Connecting, ConnectingValue>
+  | State<EngineConnectionStateType.ConnectionEstablished, void>
+  | State<EngineConnectionStateType.Disconnected, DisconnectedValue>
+
 // EngineConnection encapsulates the connection(s) to the Engine
 // for the EngineCommandManager; namely, the underlying WebSocket
 // and WebRTC connections.
@@ -55,10 +121,28 @@ class EngineConnection {
   websocket?: WebSocket
   pc?: RTCPeerConnection
   unreliableDataChannel?: RTCDataChannel
+  mediaStream?: MediaStream
+
+  private _state: EngineConnectionState = {
+    type: EngineConnectionStateType.Fresh,
+  }
+
+  get state(): EngineConnectionState {
+    return this._state
+  }
+
+  set state(next: EngineConnectionState) {
+    console.log(`${JSON.stringify(this.state)} → ${JSON.stringify(next)}`)
+    if (next.type === EngineConnectionStateType.Disconnected) {
+      console.trace()
+      const sub = next.value
+      if (sub.type === DisconnectedType.Error) {
+        console.error(sub.value)
+      }
+    }
+    this._state = next
+  }
 
-  private ready: boolean
-  private connecting: boolean
-  private dead: boolean
   private failedConnTimeout: Timeout | null
 
   readonly url: string
@@ -94,74 +178,77 @@ class EngineConnection {
   }) {
     this.url = url
     this.token = token
-    this.ready = false
-    this.connecting = false
-    this.dead = false
     this.failedConnTimeout = null
     this.onWebsocketOpen = onWebsocketOpen
     this.onDataChannelOpen = onDataChannelOpen
     this.onEngineConnectionOpen = onEngineConnectionOpen
     this.onConnectionStarted = onConnectionStarted
+
     this.onClose = onClose
     this.onNewTrack = onNewTrack
 
     // TODO(paultag): This ought to be tweakable.
     const pingIntervalMs = 10000
 
+    // Without an interval ping, our connection will timeout.
     let pingInterval = setInterval(() => {
-      if (this.dead) {
-        clearInterval(pingInterval)
-      }
-      if (this.isReady()) {
-        // When we're online, every 10 seconds, we'll attempt to put a 'ping'
-        // command through the WebSocket connection. This will help both ends
-        // of the connection maintain the TCP connection without hitting a
-        // timeout condition.
+      switch (this.state.type as EngineConnectionStateType) {
+        case EngineConnectionStateType.ConnectionEstablished:
           this.send({ type: 'ping' })
+          break
+        case EngineConnectionStateType.Disconnected:
+          clearInterval(pingInterval)
+          break
+        default:
+          break
       }
     }, pingIntervalMs)
 
     const connectionTimeoutMs = VITE_KC_CONNECTION_TIMEOUT_MS
-    let connectInterval = setInterval(() => {
-      if (this.dead) {
-        clearInterval(connectInterval)
-        return
-      }
-      if (this.isReady()) {
-        return
-      }
-      console.log('connecting via retry')
+    let connectRetryInterval = setInterval(() => {
+      if (this.state.type !== EngineConnectionStateType.Disconnected) return
+      switch (this.state.value.type) {
+        case DisconnectedType.Error:
+          clearInterval(connectRetryInterval)
+          break
+        case DisconnectedType.Timeout:
+          console.log('Trying to reconnect')
           this.connect()
+          break
+        default:
+          break
+      }
     }, connectionTimeoutMs)
   }
-  // isConnecting will return true when connect has been called, but the full
-  // WebRTC is not online.
+
   isConnecting() {
-    return this.connecting
+    return this.state.type === EngineConnectionStateType.Connecting
   }
-  // isReady will return true only when the WebRTC *and* WebSocket connection
-  // are connected. During setup, the WebSocket connection comes online first,
-  // which is used to establish the WebRTC connection. The EngineConnection
-  // is not "Ready" until both are connected.
+
   isReady() {
-    return this.ready
+    return this.state.type === EngineConnectionStateType.ConnectionEstablished
   }
+
   tearDown() {
-    this.dead = true
-    this.close()
+    this.disconnectAll()
+    this.state = {
+      type: EngineConnectionStateType.Disconnected,
+      value: { type: DisconnectedType.Quit },
     }
+  }
+
   // shouldTrace will return true when Sentry should be used to instrument
   // the Engine.
   shouldTrace() {
     return Sentry.getCurrentHub()?.getClient()?.getOptions()?.sendClientReports
   }
+
   // connect will attempt to connect to the Engine over a WebSocket, and
   // establish the WebRTC connections.
   //
   // This will attempt the full handshake, and retry if the connection
   // did not establish.
   connect() {
-    console.log('connect was called')
     if (this.isConnecting() || this.isReady()) {
       return
     }
@@ -195,228 +282,98 @@ class EngineConnection {
     let handshakeSpan: SpanPromise
     let iceSpan: SpanPromise
 
+    const spanStart = (op: string) =>
+      new SpanPromise(webrtcMediaTransaction.startChild({ op }))
+
     if (this.shouldTrace()) {
-      webrtcMediaTransaction = Sentry.startTransaction({
-        name: 'webrtc-media',
-      })
-      websocketSpan = new SpanPromise(
-        webrtcMediaTransaction.startChild({ op: 'websocket' })
-      )
+      webrtcMediaTransaction = Sentry.startTransaction({ name: 'webrtc-media' })
+      websocketSpan = spanStart('websocket')
     }
 
-    this.websocket = new WebSocket(this.url, [])
-    this.websocket.binaryType = 'arraybuffer'
-
+    const createPeerConnection = () => {
       this.pc = new RTCPeerConnection()
-    this.pc.createDataChannel('unreliable_modeling_cmds')
-    this.websocket.addEventListener('open', (event) => {
-      console.log('Connected to websocket, waiting for ICE servers')
-      if (this.token) {
-        this.send({ headers: { Authorization: `Bearer ${this.token}` } })
+
+      // Data channels MUST BE specified before SDP offers because requesting
+      // them affects what our needs are!
+      const DATACHANNEL_NAME_UMC = 'unreliable_modeling_cmds'
+      this.pc.createDataChannel(DATACHANNEL_NAME_UMC)
+
+      this.state = {
+        type: EngineConnectionStateType.Connecting,
+        value: {
+          type: ConnectingType.DataChannelRequested,
+          value: DATACHANNEL_NAME_UMC,
+        },
       }
+
+      this.pc.addEventListener('icecandidate', (event) => {
+        if (event.candidate === null) {
+          return
+        }
+
+        this.state = {
+          type: EngineConnectionStateType.Connecting,
+          value: {
+            type: ConnectingType.ICECandidateReceived,
+          },
+        }
+
+        // Request a candidate to use
+        this.send({
+          type: 'trickle_ice',
+          candidate: event.candidate.toJSON(),
+        })
       })
 
-    this.pc.addEventListener('icecandidateerror', (_event) => {
+      this.pc.addEventListener('icecandidateerror', (_event: Event) => {
         const event = _event as RTCPeerConnectionIceErrorEvent
-      console.error(
+        console.warn(
           `ICE candidate returned an error: ${event.errorCode}: ${event.errorText} for ${event.url}`
         )
       })
 
-    this.pc.addEventListener('connectionstatechange', (event) => {
-      if (this.pc?.iceConnectionState === 'connected') {
+      // https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/connectionstatechange_event
+      // Event type: generic Event type...
+      this.pc.addEventListener('connectionstatechange', (event: any) => {
+        console.log('connectionstatechange: ' + event.target?.connectionState)
+        switch (event.target?.connectionState) {
+          // From what I understand, only after have we done the ICE song and
+          // dance is it safest to connect the video tracks / stream
+          case 'connected':
             if (this.shouldTrace()) {
               iceSpan.resolve?.()
             }
-      } else if (this.pc?.iceConnectionState === 'failed') {
-        // failed is a terminal state; let's explicitly kill the
-        // connection to the server at this point.
-        console.log('failed to negotiate ice connection; restarting')
-        this.close()
+
+            // Let the browser attach to the video stream now
+            this.onNewTrack({ conn: this, mediaStream: this.mediaStream! })
+            break
+          case 'failed':
+            this.disconnectAll()
+            this.state = {
+              type: EngineConnectionStateType.Disconnected,
+              value: {
+                type: DisconnectedType.Error,
+                value: new Error(
+                  'failed to negotiate ice connection; restarting'
+                ),
+              },
             }
-    })
-
-    this.websocket.addEventListener('open', (event) => {
-      if (this.shouldTrace()) {
-        websocketSpan.resolve?.()
-
-        handshakeSpan = new SpanPromise(
-          webrtcMediaTransaction.startChild({ op: 'handshake' })
-        )
-        iceSpan = new SpanPromise(
-          webrtcMediaTransaction.startChild({ op: 'ice' })
-        )
-        dataChannelSpan = new SpanPromise(
-          webrtcMediaTransaction.startChild({
-            op: 'data-channel',
-          })
-        )
-        mediaTrackSpan = new SpanPromise(
-          webrtcMediaTransaction.startChild({
-            op: 'media-track',
-          })
-        )
-      }
-
-      if (this.shouldTrace()) {
-        Promise.all([
-          handshakeSpan.promise,
-          iceSpan.promise,
-          dataChannelSpan.promise,
-          mediaTrackSpan.promise,
-        ]).then(() => {
-          console.log('All spans finished, reporting')
-          webrtcMediaTransaction?.finish()
-        })
-      }
-
-      this.onWebsocketOpen(this)
-    })
-
-    this.websocket.addEventListener('close', (event) => {
-      console.log('websocket connection closed', event)
-      this.close()
-    })
-
-    this.websocket.addEventListener('error', (event) => {
-      console.log('websocket connection error', event)
-      this.close()
-    })
-
-    this.websocket.addEventListener('message', (event) => {
-      // In the EngineConnection, we're looking for messages to/from
-      // the server that relate to the ICE handshake, or WebRTC
-      // negotiation. There may be other messages (including ArrayBuffer
-      // messages) that are intended for the GUI itself, so be careful
-      // when assuming we're the only consumer or that all messages will
-      // be carefully formatted here.
-
-      if (typeof event.data !== 'string') {
-        return
-      }
-
-      const message: Models['WebSocketResponse_type'] = JSON.parse(event.data)
-
-      if (!message.success) {
-        const errorsString = message?.errors
-          ?.map((error) => {
-            return `  - ${error.error_code}: ${error.message}`
-          })
-          .join('\n')
-        if (message.request_id) {
-          console.error(
-            `Error in response to request ${message.request_id}:\n${errorsString}`
-          )
-        } else {
-          console.error(`Error from server:\n${errorsString}`)
-        }
-        return
-      }
-
-      let resp = message.resp
-      if (!resp) {
-        // If there's no body to the response, we can bail here.
-        return
-      }
-
-      if (resp.type === 'sdp_answer') {
-        let answer = resp.data?.answer
-        if (!answer || answer.type === 'unspecified') {
-          return
-        }
-
-        if (this.pc?.signalingState !== 'stable') {
-          // If the connection is stable, we shouldn't bother updating the
-          // SDP, since we have a stable connection to the backend. If we
-          // need to renegotiate, the whole PeerConnection needs to get
-          // tore down.
-          this.pc?.setRemoteDescription(
-            new RTCSessionDescription({
-              type: answer.type,
-              sdp: answer.sdp,
-            })
-          )
-
-          if (this.shouldTrace()) {
-            // When both ends have a local and remote SDP, we've been able to
-            // set up successfully. We'll still need to find the right ICE
-            // servers, but this is hand-shook.
-            handshakeSpan.resolve?.()
-          }
-        }
-      } else if (resp.type === 'trickle_ice') {
-        let candidate = resp.data?.candidate
-        this.pc?.addIceCandidate(candidate as RTCIceCandidateInit)
-      } else if (resp.type === 'ice_server_info' && this.pc) {
-        console.log('received ice_server_info')
-        let ice_servers = resp.data?.ice_servers
-
-        if (ice_servers?.length > 0) {
-          // When we set the Configuration, we want to always force
-          // iceTransportPolicy to 'relay', since we know the topology
-          // of the ICE/STUN/TUN server and the engine. We don't wish to
-          // talk to the engine in any configuration /other/ than relay
-          // from a infra POV.
-          this.pc.setConfiguration({
-            iceServers: ice_servers,
-            iceTransportPolicy: 'relay',
-          })
-        } else {
-          this.pc?.setConfiguration({})
-        }
-
-        // We have an ICE Servers set now. We just setConfiguration, so let's
-        // start adding things we care about to the PeerConnection and let
-        // ICE negotiation happen in the background. Everything from here
-        // until the end of this function is setup of our end of the
-        // PeerConnection and waiting for events to fire our callbacks.
-
-        this.pc.addEventListener('icecandidate', (event) => {
-          if (!this.pc || !this.websocket) return
-          if (event.candidate !== null) {
-            console.log('sending trickle ice candidate')
-            const { candidate } = event
-            this.send({
-              type: 'trickle_ice',
-              candidate: candidate.toJSON(),
-            })
-          }
-        })
-
-        // Offer to receive 1 video track
-        this.pc.addTransceiver('video', {})
-
-        // Finally (but actually firstly!), to kick things off, we're going to
-        // generate our SDP, set it on our PeerConnection, and let the server
-        // know about our capabilities.
-        this.pc
-          .createOffer()
-          .then(async (descriptionInit) => {
-            await this?.pc?.setLocalDescription(descriptionInit)
-            console.log('sent sdp_offer begin')
-            this.send({
-              type: 'sdp_offer',
-              offer: this.pc?.localDescription,
-            })
-          })
-          .catch(console.log)
-      } else if (resp.type === 'metrics_request') {
-        if (this.webrtcStatsCollector === undefined) {
-          // TODO: Error message here?
-          return
-        }
-        this.webrtcStatsCollector().then((client_metrics) => {
-          this.send({
-            type: 'metrics_response',
-            metrics: client_metrics,
-          })
-        })
+            break
+          default:
+            break
         }
       })
 
       this.pc.addEventListener('track', (event) => {
         const mediaStream = event.streams[0]
 
+        this.state = {
+          type: EngineConnectionStateType.Connecting,
+          value: {
+            type: ConnectingType.TrackReceived,
+          },
+        }
+
         if (this.shouldTrace()) {
           let mediaStreamTrack = mediaStream.getVideoTracks()[0]
           mediaStreamTrack.addEventListener('unmute', () => {
@@ -436,7 +393,7 @@ class EngineConnection {
             }
 
             let videoTrack = mediaStream.getVideoTracks()[0]
-          this.pc?.getStats(videoTrack).then((videoTrackStats) => {
+            void this.pc?.getStats(videoTrack).then((videoTrackStats) => {
               let client_metrics: ClientMetrics = {
                 rtc_frames_decoded: 0,
                 rtc_frames_dropped: 0,
@@ -481,56 +438,357 @@ class EngineConnection {
           })
         }
 
-      this.onNewTrack({
-        conn: this,
-        mediaStream: mediaStream,
-      })
+        // The app is eager to use the MediaStream; as soon as onNewTrack is
+        // called, the following sequence happens:
+        // EngineConnection.onNewTrack -> StoreState.setMediaStream ->
+        // Stream.tsx reacts to mediaStream change, setting a video element.
+        // We wait until connectionstatechange changes to "connected"
+        // to pass it to the rest of the application.
+
+        this.mediaStream = mediaStream
       })
 
       this.pc.addEventListener('datachannel', (event) => {
         this.unreliableDataChannel = event.channel
 
-      console.log('accepted unreliable data channel', event.channel.label)
+        this.state = {
+          type: EngineConnectionStateType.Connecting,
+          value: {
+            type: ConnectingType.DataChannelConnecting,
+            value: event.channel.label,
+          },
+        }
+
         this.unreliableDataChannel.addEventListener('open', (event) => {
-        console.log('unreliable data channel opened', event)
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.DataChannelEstablished,
+            },
+          }
+
           if (this.shouldTrace()) {
             dataChannelSpan.resolve?.()
           }
 
           this.onDataChannelOpen(this)
 
-        this.ready = true
-        this.connecting = false
-        // Do this after we set the connection is ready to avoid errors when
-        // we try to send messages before the connection is ready.
+          // Everything is now connected.
+          this.state = { type: EngineConnectionStateType.ConnectionEstablished }
+
           this.onEngineConnectionOpen(this)
         })
 
         this.unreliableDataChannel.addEventListener('close', (event) => {
+          console.log(event)
           console.log('unreliable data channel closed')
-        this.close()
+          this.disconnectAll()
+          this.unreliableDataChannel = undefined
+
+          if (this.areAllConnectionsClosed()) {
+            this.state = {
+              type: EngineConnectionStateType.Disconnected,
+              value: { type: DisconnectedType.Quit },
+            }
+          }
         })
 
         this.unreliableDataChannel.addEventListener('error', (event) => {
-        console.log('unreliable data channel error')
-        this.close()
+          this.disconnectAll()
+
+          this.state = {
+            type: EngineConnectionStateType.Disconnected,
+            value: {
+              type: DisconnectedType.Error,
+              value: new Error(event.toString()),
+            },
+          }
         })
       })
+    }
+
+    this.state = {
+      type: EngineConnectionStateType.Connecting,
+      value: {
+        type: ConnectingType.WebSocketConnecting,
+      },
+    }
+
+    this.websocket = new WebSocket(this.url, [])
+    this.websocket.binaryType = 'arraybuffer'
+
+    this.websocket.addEventListener('open', (event) => {
+      this.state = {
+        type: EngineConnectionStateType.Connecting,
+        value: {
+          type: ConnectingType.WebSocketEstablished,
+        },
+      }
+
+      this.onWebsocketOpen(this)
+
+      // This is required for when KCMA is running stand-alone / within Tauri.
+      // Otherwise when run in a browser, the token is sent implicitly via
+      // the Cookie header.
+      if (this.token) {
+        this.send({ headers: { Authorization: `Bearer ${this.token}` } })
+      }
+
+      if (this.shouldTrace()) {
+        websocketSpan.resolve?.()
+
+        handshakeSpan = spanStart('handshake')
+        iceSpan = spanStart('ice')
+        dataChannelSpan = spanStart('data-channel')
+        mediaTrackSpan = spanStart('media-track')
+      }
+
+      if (this.shouldTrace()) {
+        void Promise.all([
+          handshakeSpan.promise,
+          iceSpan.promise,
+          dataChannelSpan.promise,
+          mediaTrackSpan.promise,
+        ]).then(() => {
+          console.log('All spans finished, reporting')
+          webrtcMediaTransaction?.finish()
+        })
+      }
+    })
+
+    this.websocket.addEventListener('close', (event) => {
+      this.disconnectAll()
+      this.websocket = undefined
+
+      if (this.areAllConnectionsClosed()) {
+        this.state = {
+          type: EngineConnectionStateType.Disconnected,
+          value: { type: DisconnectedType.Quit },
+        }
+      }
+    })
+
+    this.websocket.addEventListener('error', (event) => {
+      this.disconnectAll()
+
+      this.state = {
+        type: EngineConnectionStateType.Disconnected,
+        value: {
+          type: DisconnectedType.Error,
+          value: new Error(event.toString()),
+        },
+      }
+    })
+
+    this.websocket.addEventListener('message', (event) => {
+      // In the EngineConnection, we're looking for messages to/from
+      // the server that relate to the ICE handshake, or WebRTC
+      // negotiation. There may be other messages (including ArrayBuffer
+      // messages) that are intended for the GUI itself, so be careful
+      // when assuming we're the only consumer or that all messages will
+      // be carefully formatted here.
+
+      if (typeof event.data !== 'string') {
+        return
+      }
+
+      const message: Models['WebSocketResponse_type'] = JSON.parse(event.data)
+
+      if (!message.success) {
+        const errorsString = message?.errors
+          ?.map((error) => {
+            return `  - ${error.error_code}: ${error.message}`
+          })
+          .join('\n')
+        if (message.request_id) {
+          console.error(
+            `Error in response to request ${message.request_id}:\n${errorsString}`
+          )
+        } else {
+          console.error(`Error from server:\n${errorsString}`)
+        }
+        return
+      }
+
+      let resp = message.resp
+
+      // If there's no body to the response, we can bail here.
+      // !resp.type is usually "pong" response for our "ping"
+      if (!resp || !resp.type) {
+        return
+      }
+
+      console.log('received', resp)
+
+      switch (resp.type) {
+        case 'ice_server_info':
+          let ice_servers = resp.data?.ice_servers
+
+          // Now that we have some ICE servers it makes sense
+          // to start initializing the RTCPeerConnection. RTCPeerConnection
+          // will begin the ICE process.
+          createPeerConnection()
+
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.PeerConnectionCreated,
+            },
+          }
+
+          // No ICE servers can be valid in a local dev. env.
+          if (ice_servers?.length === 0) {
+            console.warn('No ICE servers')
+            this.pc?.setConfiguration({})
+          } else {
+            // When we set the Configuration, we want to always force
+            // iceTransportPolicy to 'relay', since we know the topology
+            // of the ICE/STUN/TUN server and the engine. We don't wish to
+            // talk to the engine in any configuration /other/ than relay
+            // from a infra POV.
+            this.pc?.setConfiguration({
+              iceServers: ice_servers,
+              iceTransportPolicy: 'relay',
+            })
+          }
+
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.ICEServersSet,
+            },
+          }
+
+          // We have an ICE Servers set now. We just setConfiguration, so let's
+          // start adding things we care about to the PeerConnection and let
+          // ICE negotiation happen in the background. Everything from here
+          // until the end of this function is setup of our end of the
+          // PeerConnection and waiting for events to fire our callbacks.
+
+          // Add a transceiver to our SDP offer
+          this.pc?.addTransceiver('video', {
+            direction: 'recvonly',
+          })
+
+          // Create a session description offer based on our local environment
+          // that we will send to the remote end. The remote will send back
+          // what it supports via sdp_answer.
+          this.pc
+            ?.createOffer()
+            .then((offer: RTCSessionDescriptionInit) => {
+              console.log(offer)
+              this.state = {
+                type: EngineConnectionStateType.Connecting,
+                value: {
+                  type: ConnectingType.SetLocalDescription,
+                },
+              }
+              return this.pc?.setLocalDescription(offer).then(() => {
+                this.send({
+                  type: 'sdp_offer',
+                  offer,
+                })
+                this.state = {
+                  type: EngineConnectionStateType.Connecting,
+                  value: {
+                    type: ConnectingType.OfferedSdp,
+                  },
+                }
+              })
+            })
+            .catch((error: Error) => {
+              console.error(error)
+              // The local description is invalid, so there's no point continuing.
+              this.disconnectAll()
+              this.state = {
+                type: EngineConnectionStateType.Disconnected,
+                value: {
+                  type: DisconnectedType.Error,
+                  value: error,
+                },
+              }
+            })
+          break
+
+        case 'sdp_answer':
+          let answer = resp.data?.answer
+          if (!answer || answer.type === 'unspecified') {
+            return
+          }
+
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.ReceivedSdp,
+            },
+          }
+
+          // As soon as this is set, RTCPeerConnection tries to
+          // establish a connection.
+          // @ts-ignore
+          // Have to ignore because dom.ts doesn't have the right type
+          void this.pc?.setRemoteDescription(answer)
+
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.SetRemoteDescription,
+            },
+          }
+
+          this.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.WebRTCConnecting,
+            },
+          }
+
+          if (this.shouldTrace()) {
+            // When both ends have a local and remote SDP, we've been able to
+            // set up successfully. We'll still need to find the right ICE
+            // servers, but this is hand-shook.
+            handshakeSpan.resolve?.()
+          }
+          break
+
+        case 'trickle_ice':
+          let candidate = resp.data?.candidate
+          console.log('trickle_ice: using this candidate: ', candidate)
+          void this.pc?.addIceCandidate(candidate as RTCIceCandidateInit)
+          break
+
+        case 'metrics_request':
+          if (this.webrtcStatsCollector === undefined) {
+            // TODO: Error message here?
+            return
+          }
+          void this.webrtcStatsCollector().then((client_metrics) => {
+            this.send({
+              type: 'metrics_response',
+              metrics: client_metrics,
+            })
+          })
+          break
+      }
+    })
 
     const connectionTimeoutMs = VITE_KC_CONNECTION_TIMEOUT_MS
-
     if (this.failedConnTimeout) {
-      console.log('clearing timeout before set')
       clearTimeout(this.failedConnTimeout)
       this.failedConnTimeout = null
     }
-    console.log('timeout set')
     this.failedConnTimeout = setTimeout(() => {
       if (this.isReady()) {
         return
       }
-      console.log('engine connection timeout on connection, closing')
-      this.close()
+      this.failedConnTimeout = null
+      this.disconnectAll()
+      this.state = {
+        type: EngineConnectionStateType.Disconnected,
+        value: {
+          type: DisconnectedType.Timeout,
+        },
+      }
     }, connectionTimeoutMs)
 
     this.onConnectionStarted(this)
@@ -549,23 +807,15 @@ class EngineConnection {
       typeof message === 'string' ? message : JSON.stringify(message)
     )
   }
-  close() {
+  disconnectAll() {
     this.websocket?.close()
-    this.pc?.close()
     this.unreliableDataChannel?.close()
-    this.websocket = undefined
-    this.pc = undefined
-    this.unreliableDataChannel = undefined
+    this.pc?.close()
     this.webrtcStatsCollector = undefined
-    if (this.failedConnTimeout) {
-      console.log('closed timeout in close')
-      clearTimeout(this.failedConnTimeout)
-      this.failedConnTimeout = null
   }
-
-    this.onClose(this)
-    this.ready = false
-    this.connecting = false
+  areAllConnectionsClosed() {
+    console.log(this.websocket, this.pc, this.unreliableDataChannel)
+    return !this.websocket && !this.pc && !this.unreliableDataChannel
   }
 }
 
@@ -685,7 +935,7 @@ export class EngineCommandManager {
         // We also do this here because we want to ensure we create the gizmo
         // and execute the code everytime the stream is restarted.
         const gizmoId = uuidv4()
-        this.sendSceneCommand({
+        void this.sendSceneCommand({
           type: 'modeling_cmd_req',
           cmd_id: gizmoId,
           cmd: {
@@ -698,7 +948,7 @@ export class EngineCommandManager {
         })
 
         // Initialize the planes.
-        this.initPlanes().then(() => {
+        void this.initPlanes().then(() => {
           // We execute the code here to make sure if the stream was to
           // restart in a session, we want to make sure to execute the code.
           // We force it to re-execute the code because we want to make sure
@@ -745,7 +995,7 @@ export class EngineCommandManager {
             // because in all other cases we send JSON strings. But in the case of
             // export we send a binary blob.
             // Pass this to our export function.
-            exportSave(event.data)
+            void exportSave(event.data)
           } else {
             const message: Models['WebSocketResponse_type'] = JSON.parse(
               event.data

src/routes/Onboarding/CodeEditor.tsx

@@ -29,7 +29,7 @@ export default function CodeEditor() {
             The left pane is where you write your code. It's a code editor with
             syntax highlighting and autocompletion. We've decided to take the
             difficult route of writing our own language—called <code>kcl</code>
-            —for describing geometry, because don't want to inherit all the
+            —for describing geometry, because we don't want to inherit all the
             other functionality from existing languages. We have a lot of ideas
             about how <code>kcl</code> will evolve, and we want to hear your
             thoughts on it.

src/wasm-lib/Cargo.lock

@@ -1441,9 +1441,9 @@ dependencies = [
 
 [[package]]
 name = "h2"
-version = "0.3.21"
+version = "0.3.24"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "91fc23aa11be92976ef4729127f1a74adf36d8436f7816b185d18df956790833"
+checksum = "bb2c4422095b67ee78da96fbb51a4cc413b3b25883c7717ff7ca1ab31022c9c9"
 dependencies = [
  "bytes",
  "fnv",
@@ -1451,7 +1451,7 @@ dependencies = [
  "futures-sink",
  "futures-util",
  "http 0.2.9",
- "indexmap 1.9.3",
+ "indexmap 2.0.2",
  "slab",
  "tokio",
  "tokio-util",
@@ -1864,6 +1864,7 @@ dependencies = [
  "dashmap",
  "databake",
  "derive-docs 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
+ "either",
  "expectorate",
  "futures",
  "insta",
@@ -1943,7 +1944,7 @@ dependencies = [
 [[package]]
 name = "kittycad-execution-plan"
 version = "0.1.0"
-source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f"
+source = "git+https://github.com/KittyCAD/modeling-api?branch=main#935256e4a7080ea130b09b578e16820dc96e78e4"
 dependencies = [
  "bytes",
  "insta",
@@ -1972,7 +1973,7 @@ dependencies = [
 [[package]]
 name = "kittycad-execution-plan-macros"
 version = "0.1.2"
-source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f"
+source = "git+https://github.com/KittyCAD/modeling-api?branch=main#935256e4a7080ea130b09b578e16820dc96e78e4"
 dependencies = [
  "proc-macro2",
  "quote",
@@ -1992,8 +1993,8 @@ dependencies = [
 
 [[package]]
 name = "kittycad-modeling-cmds"
-version = "0.1.11"
-source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f"
+version = "0.1.12"
+source = "git+https://github.com/KittyCAD/modeling-api?branch=main#935256e4a7080ea130b09b578e16820dc96e78e4"
 dependencies = [
  "anyhow",
  "chrono",
@@ -2020,7 +2021,7 @@ dependencies = [
 [[package]]
 name = "kittycad-modeling-session"
 version = "0.1.0"
-source = "git+https://github.com/KittyCAD/modeling-api?branch=main#9200b9540fa5ae99b692db276c625223116f467f"
+source = "git+https://github.com/KittyCAD/modeling-api?branch=main#935256e4a7080ea130b09b578e16820dc96e78e4"
 dependencies = [
  "futures",
  "kittycad",

src/wasm-lib/grackle/src/binding_scope.rs

@@ -2,11 +2,10 @@ use kcl_lib::ast::types::LiteralIdentifier;
 use kcl_lib::ast::types::LiteralValue;
 
 use crate::CompileError;
+use crate::KclFunction;
 
 use super::native_functions;
 use super::Address;
-use super::KclFunction;
-use super::String2;
 
 use std::collections::HashMap;
 
@@ -21,6 +20,14 @@ pub enum EpBinding {
     Sequence(Vec<EpBinding>),
     /// A sequence of KCL values, indexed by their identifier.
     Map(HashMap<String, EpBinding>),
+    /// Not associated with a KCEP address.
+    Function(KclFunction),
+}
+
+impl From<KclFunction> for EpBinding {
+    fn from(f: KclFunction) -> Self {
+        Self::Function(f)
+    }
 }
 
 impl EpBinding {
@@ -31,16 +38,18 @@ impl EpBinding {
             LiteralIdentifier::Literal(litval) => match litval.value {
                 // Arrays can be indexed by integers.
                 LiteralValue::IInteger(i) => match self {
-                    EpBinding::Single(_) => Err(CompileError::CannotIndex),
                     EpBinding::Sequence(seq) => {
                         let i = usize::try_from(i).map_err(|_| CompileError::InvalidIndex(i.to_string()))?;
                         seq.get(i).ok_or(CompileError::IndexOutOfBounds { i, len: seq.len() })
                     }
                     EpBinding::Map(_) => Err(CompileError::CannotIndex),
+                    EpBinding::Single(_) => Err(CompileError::CannotIndex),
+                    EpBinding::Function(_) => Err(CompileError::CannotIndex),
                 },
                 // Objects can be indexed by string properties.
                 LiteralValue::String(property) => match self {
                     EpBinding::Single(_) => Err(CompileError::NoProperties),
+                    EpBinding::Function(_) => Err(CompileError::NoProperties),
                     EpBinding::Sequence(_) => Err(CompileError::ArrayDoesNotHaveProperties),
                     EpBinding::Map(map) => map.get(&property).ok_or(CompileError::UndefinedProperty { property }),
                 },
@@ -67,7 +76,6 @@ pub struct BindingScope {
     // KCL value which are stored in EP memory.
     ep_bindings: HashMap<String, EpBinding>,
     /// KCL functions. They do NOT get stored in EP memory.
-    function_bindings: HashMap<String2, Box<dyn KclFunction>>,
     parent: Option<Box<BindingScope>>,
 }
 
@@ -80,29 +88,39 @@ impl BindingScope {
         Self {
             // TODO: Actually put the stdlib prelude in here,
             // things like `startSketchAt` and `line`.
-            function_bindings: HashMap::from([
-                ("id".into(), Box::new(native_functions::Id) as _),
-                ("add".into(), Box::new(native_functions::Add) as _),
+            ep_bindings: HashMap::from([
+                ("id".into(), EpBinding::from(KclFunction::Id(native_functions::Id))),
+                ("add".into(), EpBinding::from(KclFunction::Add(native_functions::Add))),
+                (
+                    "startSketchAt".into(),
+                    EpBinding::from(KclFunction::StartSketchAt(native_functions::StartSketchAt)),
+                ),
             ]),
-            ep_bindings: Default::default(),
             parent: None,
         }
     }
 
     /// Add a new scope, e.g. for new function calls.
-    #[allow(dead_code)] // TODO: when we implement function expressions.
-    pub fn add_scope(self) -> Self {
-        Self {
-            function_bindings: Default::default(),
-            ep_bindings: Default::default(),
-            parent: Some(Box::new(self)),
-        }
+    pub fn add_scope(&mut self) {
+        // Move all data from `self` into `this`.
+        let this_parent = self.parent.take();
+        let this_ep_bindings = self.ep_bindings.drain().collect();
+        let this = Self {
+            ep_bindings: this_ep_bindings,
+            parent: this_parent,
+        };
+        // Turn `self` into a new scope, with the old `self` as its parent.
+        self.parent = Some(Box::new(this));
     }
 
     //// Remove a scope, e.g. when exiting a function call.
-    #[allow(dead_code)] // TODO: when we implement function expressions.
-    pub fn remove_scope(self) -> Self {
-        *self.parent.unwrap()
+    pub fn remove_scope(&mut self) {
+        // The scope is finished, so erase all its local variables.
+        self.ep_bindings.clear();
+        // Pop the stack -- the parent scope is now the current scope.
+        let p = self.parent.take().expect("cannot remove the root scope");
+        self.parent = p.parent;
+        self.ep_bindings = p.ep_bindings;
     }
 
     /// Add a binding (e.g. defining a new variable)
@@ -126,10 +144,11 @@ impl BindingScope {
 
     /// Look up a function bound to the given identifier.
     pub fn get_fn(&self, identifier: &str) -> GetFnResult {
-        if let Some(f) = self.function_bindings.get(identifier) {
-            GetFnResult::Found(f.as_ref())
-        } else if self.get(identifier).is_some() {
-            GetFnResult::NonCallable
+        if let Some(x) = self.get(identifier) {
+            match x {
+                EpBinding::Function(f) => GetFnResult::Found(f),
+                _ => GetFnResult::NonCallable,
+            }
         } else if let Some(ref parent) = self.parent {
             parent.get_fn(identifier)
         } else {
@@ -139,7 +158,7 @@ impl BindingScope {
 }
 
 pub enum GetFnResult<'a> {
-    Found(&'a dyn KclFunction),
+    Found(&'a KclFunction),
     NonCallable,
     NotFound,
 }

src/wasm-lib/grackle/src/error.rs

@@ -0,0 +1,56 @@
+use kcl_lib::ast::types::RequiredParamAfterOptionalParam;
+use kittycad_execution_plan::ExecutionError;
+
+use crate::String2;
+
+#[derive(Debug, thiserror::Error, PartialEq, Clone)]
+pub enum CompileError {
+    #[error("the name {name} was not defined")]
+    Undefined { name: String },
+    #[error("the function {fn_name} requires at least {required} arguments but you only supplied {actual}")]
+    NotEnoughArgs {
+        fn_name: String2,
+        required: usize,
+        actual: usize,
+    },
+    #[error("the function {fn_name} accepts at most {maximum} arguments but you supplied {actual}")]
+    TooManyArgs {
+        fn_name: String2,
+        maximum: usize,
+        actual: usize,
+    },
+    #[error("you tried to call {name} but it's not a function")]
+    NotCallable { name: String },
+    #[error("you're trying to use an operand that isn't compatible with the given arithmetic operator: {0}")]
+    InvalidOperand(&'static str),
+    #[error("you cannot use the value {0} as an index")]
+    InvalidIndex(String),
+    #[error("you tried to index into a value that isn't an array. Only arrays have numeric indices!")]
+    CannotIndex,
+    #[error("you tried to get the element {i} but that index is out of bounds. The array only has a length of {len}")]
+    IndexOutOfBounds { i: usize, len: usize },
+    #[error("you tried to access the property of a value that doesn't have any properties")]
+    NoProperties,
+    #[error("you tried to access a property of an array, but arrays don't have properties. They do have numeric indexes though, try using an index e.g. [0]")]
+    ArrayDoesNotHaveProperties,
+    #[error(
+        "you tried to read the '.{property}' of an object, but the object doesn't have any properties with that key"
+    )]
+    UndefinedProperty { property: String },
+    #[error("{0}")]
+    BadParamOrder(RequiredParamAfterOptionalParam),
+    #[error("A KCL function cannot have anything after its return value")]
+    MultipleReturns,
+    #[error("A KCL function must end with a return statement, but your function doesn't have one.")]
+    NoReturnStmt,
+    #[error("You used the %, which means \"substitute this argument for the value to the left in this |> pipeline\". But there is no such value, because you're not calling a pipeline.")]
+    NotInPipeline,
+}
+
+#[derive(Debug, thiserror::Error)]
+pub enum Error {
+    #[error("{0}")]
+    Compile(#[from] CompileError),
+    #[error("{0}")]
+    Execution(#[from] ExecutionError),
+}

src/wasm-lib/grackle/src/kcl_value_group.rs

@@ -9,6 +9,7 @@ pub enum KclValueGroup {
     ObjectExpression(Box<ast::types::ObjectExpression>),
 }
 
+#[derive(Debug)]
 pub enum SingleValue {
     Literal(Box<ast::types::Literal>),
     Identifier(Box<ast::types::Identifier>),
@@ -18,6 +19,8 @@ pub enum SingleValue {
     UnaryExpression(Box<ast::types::UnaryExpression>),
     KclNoneExpression(ast::types::KclNone),
     MemberExpression(Box<ast::types::MemberExpression>),
+    FunctionExpression(Box<ast::types::FunctionExpression>),
+    PipeSubstitution(Box<ast::types::PipeSubstitution>),
 }
 
 impl From<ast::types::BinaryPart> for KclValueGroup {
@@ -59,7 +62,8 @@ impl From<ast::types::Value> for KclValueGroup {
             ast::types::Value::ArrayExpression(e) => Self::ArrayExpression(e),
             ast::types::Value::ObjectExpression(e) => Self::ObjectExpression(e),
             ast::types::Value::MemberExpression(e) => Self::Single(SingleValue::MemberExpression(e)),
-            ast::types::Value::PipeSubstitution(_) | ast::types::Value::FunctionExpression(_) => todo!(),
+            ast::types::Value::FunctionExpression(e) => Self::Single(SingleValue::FunctionExpression(e)),
+            ast::types::Value::PipeSubstitution(e) => Self::Single(SingleValue::PipeSubstitution(e)),
         }
     }
 }
@@ -76,6 +80,8 @@ impl From<KclValueGroup> for ast::types::Value {
                 SingleValue::UnaryExpression(e) => ast::types::Value::UnaryExpression(e),
                 SingleValue::KclNoneExpression(e) => ast::types::Value::None(e),
                 SingleValue::MemberExpression(e) => ast::types::Value::MemberExpression(e),
+                SingleValue::FunctionExpression(e) => ast::types::Value::FunctionExpression(e),
+                SingleValue::PipeSubstitution(e) => ast::types::Value::PipeSubstitution(e),
             },
             KclValueGroup::ArrayExpression(e) => ast::types::Value::ArrayExpression(e),
             KclValueGroup::ObjectExpression(e) => ast::types::Value::ObjectExpression(e),

src/wasm-lib/grackle/src/lib.rs

@@ -1,4 +1,5 @@
 mod binding_scope;
+mod error;
 mod kcl_value_group;
 mod native_functions;
 #[cfg(test)]
@@ -8,21 +9,22 @@ use std::collections::HashMap;
 
 use kcl_lib::{
     ast,
-    ast::types::{BodyItem, KclNone, LiteralValue, Program},
+    ast::types::{BodyItem, FunctionExpressionParts, KclNone, LiteralValue, Program},
 };
 use kittycad_execution_plan as ep;
-use kittycad_execution_plan::{Address, ExecutionError, Instruction};
+use kittycad_execution_plan::{Address, Instruction};
 use kittycad_execution_plan_traits as ept;
 use kittycad_execution_plan_traits::NumericPrimitive;
 use kittycad_modeling_session::Session;
 
 use self::binding_scope::{BindingScope, EpBinding, GetFnResult};
+use self::error::{CompileError, Error};
 use self::kcl_value_group::{KclValueGroup, SingleValue};
 
 /// Execute a KCL program by compiling into an execution plan, then running that.
 pub async fn execute(ast: Program, session: Session) -> Result<(), Error> {
     let mut planner = Planner::new();
-    let plan = planner.build_plan(ast)?;
+    let (plan, _retval) = planner.build_plan(ast)?;
     let mut mem = kittycad_execution_plan::Memory::default();
     kittycad_execution_plan::execute(&mut mem, plan, session).await?;
     Ok(())
@@ -44,25 +46,42 @@ impl Planner {
         }
     }
 
-    fn build_plan(&mut self, program: Program) -> PlanRes {
-        program.body.into_iter().try_fold(Vec::new(), |mut instructions, item| {
+    /// If successful, return the KCEP instructions for executing the given program.
+    /// If the program is a function with a return, then it also returns the KCL function's return value.
+    fn build_plan(&mut self, program: Program) -> Result<(Vec<Instruction>, Option<EpBinding>), CompileError> {
+        program
+            .body
+            .into_iter()
+            .try_fold((Vec::new(), None), |(mut instructions, mut retval), item| {
+                if retval.is_some() {
+                    return Err(CompileError::MultipleReturns);
+                }
+                let mut ctx = Context::default();
                 let instructions_for_this_node = match item {
                     BodyItem::ExpressionStatement(node) => match KclValueGroup::from(node.expression) {
-                    KclValueGroup::Single(value) => self.plan_to_compute_single(value)?.instructions,
+                        KclValueGroup::Single(value) => self.plan_to_compute_single(&mut ctx, value)?.instructions,
                         KclValueGroup::ArrayExpression(_) => todo!(),
                         KclValueGroup::ObjectExpression(_) => todo!(),
                     },
                     BodyItem::VariableDeclaration(node) => self.plan_to_bind(node)?,
-                BodyItem::ReturnStatement(_) => todo!(),
+                    BodyItem::ReturnStatement(node) => match KclValueGroup::from(node.argument) {
+                        KclValueGroup::Single(value) => {
+                            let EvalPlan { instructions, binding } = self.plan_to_compute_single(&mut ctx, value)?;
+                            retval = Some(binding);
+                            instructions
+                        }
+                        KclValueGroup::ArrayExpression(_) => todo!(),
+                        KclValueGroup::ObjectExpression(_) => todo!(),
+                    },
                 };
                 instructions.extend(instructions_for_this_node);
-            Ok(instructions)
+                Ok((instructions, retval))
             })
     }
 
     /// Emits instructions which, when run, compute a given KCL value and store it in memory.
     /// Returns the instructions, and the destination address of the value.
-    fn plan_to_compute_single(&mut self, value: SingleValue) -> Result<EvalPlan, CompileError> {
+    fn plan_to_compute_single(&mut self, ctx: &mut Context, value: SingleValue) -> Result<EvalPlan, CompileError> {
         match value {
             SingleValue::KclNoneExpression(KclNone { start: _, end: _ }) => {
                 let address = self.next_addr.offset_by(1);
@@ -74,6 +93,23 @@ impl Planner {
                     binding: EpBinding::Single(address),
                 })
             }
+            SingleValue::FunctionExpression(expr) => {
+                let FunctionExpressionParts {
+                    start: _,
+                    end: _,
+                    params_required,
+                    params_optional,
+                    body,
+                } = expr.into_parts().map_err(CompileError::BadParamOrder)?;
+                Ok(EvalPlan {
+                    instructions: Vec::new(),
+                    binding: EpBinding::from(KclFunction::UserDefined(UserDefinedFunction {
+                        params_optional,
+                        params_required,
+                        body,
+                    })),
+                })
+            }
             SingleValue::Literal(expr) => {
                 let kcep_val = kcl_literal_to_kcep_literal(expr.value);
                 // KCEP primitives always have size of 1, because each address holds 1 primitive.
@@ -88,7 +124,27 @@ impl Planner {
                 })
             }
             SingleValue::Identifier(expr) => {
-                // This is just duplicating a binding.
+                // The KCL parser interprets bools as identifiers.
+                // Consider changing them to be KCL literals instead.
+                let b = if expr.name == "true" {
+                    Some(true)
+                } else if expr.name == "false" {
+                    Some(false)
+                } else {
+                    None
+                };
+                if let Some(b) = b {
+                    let address = self.next_addr.offset_by(1);
+                    return Ok(EvalPlan {
+                        instructions: vec![Instruction::SetPrimitive {
+                            address,
+                            value: ept::Primitive::Bool(b),
+                        }],
+                        binding: EpBinding::Single(address),
+                    });
+                }
+
+                // This identifier is just duplicating a binding.
                 // So, don't emit any instructions, because the value has already been computed.
                 // Just return the address that it was stored at after being computed.
                 let previously_bound_to = self
@@ -100,9 +156,33 @@ impl Planner {
                     binding: previously_bound_to.clone(),
                 })
             }
+            SingleValue::UnaryExpression(expr) => {
+                let operand = self.plan_to_compute_single(ctx, SingleValue::from(expr.argument))?;
+                let EpBinding::Single(binding) = operand.binding else {
+                    return Err(CompileError::InvalidOperand(
+                        "you tried to use a composite value (e.g. array or object) as the operand to some math",
+                    ));
+                };
+                let destination = self.next_addr.offset_by(1);
+                let mut plan = operand.instructions;
+                plan.push(Instruction::UnaryArithmetic {
+                    arithmetic: ep::UnaryArithmetic {
+                        operation: match expr.operator {
+                            ast::types::UnaryOperator::Neg => ep::UnaryOperation::Neg,
+                            ast::types::UnaryOperator::Not => ep::UnaryOperation::Not,
+                        },
+                        operand: ep::Operand::Reference(binding),
+                    },
+                    destination,
+                });
+                Ok(EvalPlan {
+                    instructions: plan,
+                    binding: EpBinding::Single(destination),
+                })
+            }
             SingleValue::BinaryExpression(expr) => {
-                let l = self.plan_to_compute_single(SingleValue::from(expr.left))?;
-                let r = self.plan_to_compute_single(SingleValue::from(expr.right))?;
+                let l = self.plan_to_compute_single(ctx, SingleValue::from(expr.left))?;
+                let r = self.plan_to_compute_single(ctx, SingleValue::from(expr.right))?;
                 let EpBinding::Single(l_binding) = l.binding else {
                     return Err(CompileError::InvalidOperand(
                         "you tried to use a composite value (e.g. array or object) as the operand to some math",
@@ -117,13 +197,13 @@ impl Planner {
                 let mut plan = Vec::with_capacity(l.instructions.len() + r.instructions.len() + 1);
                 plan.extend(l.instructions);
                 plan.extend(r.instructions);
-                plan.push(Instruction::Arithmetic {
-                    arithmetic: ep::Arithmetic {
+                plan.push(Instruction::BinaryArithmetic {
+                    arithmetic: ep::BinaryArithmetic {
                         operation: match expr.operator {
-                            ast::types::BinaryOperator::Add => ep::Operation::Add,
-                            ast::types::BinaryOperator::Sub => ep::Operation::Sub,
-                            ast::types::BinaryOperator::Mul => ep::Operation::Mul,
-                            ast::types::BinaryOperator::Div => ep::Operation::Div,
+                            ast::types::BinaryOperator::Add => ep::BinaryOperation::Add,
+                            ast::types::BinaryOperator::Sub => ep::BinaryOperation::Sub,
+                            ast::types::BinaryOperator::Mul => ep::BinaryOperation::Mul,
+                            ast::types::BinaryOperator::Div => ep::BinaryOperation::Div,
                             ast::types::BinaryOperator::Mod => {
                                 todo!("execution plan instruction set doesn't support Mod yet")
                             }
@@ -142,6 +222,7 @@ impl Planner {
                 })
             }
             SingleValue::CallExpression(expr) => {
+                // Make a plan to compute all the arguments to this call.
                 let (mut instructions, args) = expr.arguments.into_iter().try_fold(
                     (Vec::new(), Vec::new()),
                     |(mut acc_instrs, mut acc_args), argument| {
@@ -149,7 +230,7 @@ impl Planner {
                             instructions: new_instructions,
                             binding: arg,
                         } = match KclValueGroup::from(argument) {
-                            KclValueGroup::Single(value) => self.plan_to_compute_single(value)?,
+                            KclValueGroup::Single(value) => self.plan_to_compute_single(ctx, value)?,
                             KclValueGroup::ArrayExpression(_) => todo!(),
                             KclValueGroup::ObjectExpression(_) => todo!(),
                         };
@@ -158,6 +239,7 @@ impl Planner {
                         Ok((acc_instrs, acc_args))
                     },
                 )?;
+                // Look up the function being called.
                 let callee = match self.binding_scope.get_fn(&expr.callee.name) {
                     GetFnResult::Found(f) => f,
                     GetFnResult::NonCallable => {
@@ -172,10 +254,67 @@ impl Planner {
                     }
                 };
 
+                // Emit instructions to call that function with the given arguments.
+                use native_functions::Callable;
                 let EvalPlan {
                     instructions: eval_instrs,
                     binding,
-                } = callee.call(&mut self.next_addr, args)?;
+                } = match callee {
+                    KclFunction::Id(f) => f.call(&mut self.next_addr, args)?,
+                    KclFunction::StartSketchAt(f) => f.call(&mut self.next_addr, args)?,
+                    KclFunction::Add(f) => f.call(&mut self.next_addr, args)?,
+                    KclFunction::UserDefined(f) => {
+                        let UserDefinedFunction {
+                            params_optional,
+                            params_required,
+                            body: function_body,
+                        } = f.clone();
+                        let num_required_params = params_required.len();
+                        self.binding_scope.add_scope();
+
+                        // Bind the call's arguments to the names of the function's parameters.
+                        let num_actual_params = args.len();
+                        let mut arg_iter = args.into_iter();
+                        let max_params = params_required.len() + params_optional.len();
+                        if num_actual_params > max_params {
+                            return Err(CompileError::TooManyArgs {
+                                fn_name: "".into(),
+                                maximum: max_params,
+                                actual: num_actual_params,
+                            });
+                        }
+
+                        // Bind required parameters
+                        for param in params_required {
+                            let arg = arg_iter.next().ok_or(CompileError::NotEnoughArgs {
+                                fn_name: "".into(),
+                                required: num_required_params,
+                                actual: num_actual_params,
+                            })?;
+                            self.binding_scope.bind(param.identifier.name, arg);
+                        }
+
+                        // Bind optional parameters
+                        for param in params_optional {
+                            let Some(arg) = arg_iter.next() else {
+                                break;
+                            };
+                            self.binding_scope.bind(param.identifier.name, arg);
+                        }
+
+                        let (instructions, retval) = self.build_plan(function_body)?;
+                        let Some(retval) = retval else {
+                            return Err(CompileError::NoReturnStmt);
+                        };
+                        self.binding_scope.remove_scope();
+                        EvalPlan {
+                            instructions,
+                            binding: retval,
+                        }
+                    }
+                };
+
+                // Combine the "evaluate arguments" plan with the "call function" plan.
                 instructions.extend(eval_instrs);
                 Ok(EvalPlan { instructions, binding })
             }
@@ -207,8 +346,56 @@ impl Planner {
                     binding: binding.clone(),
                 })
             }
-            SingleValue::PipeExpression(_) => todo!(),
-            SingleValue::UnaryExpression(_) => todo!(),
+            SingleValue::PipeSubstitution(_expr) => {
+                if let Some(ref binding) = ctx.pipe_substitution {
+                    Ok(EvalPlan {
+                        instructions: Vec::new(),
+                        binding: binding.clone(),
+                    })
+                } else {
+                    Err(CompileError::NotInPipeline)
+                }
+            }
+            SingleValue::PipeExpression(expr) => {
+                let mut bodies = expr.body.into_iter();
+
+                // Get the first expression (i.e. body) of the pipeline.
+                let first = bodies.next().expect("Pipe expression must have > 1 item");
+                let EvalPlan {
+                    mut instructions,
+                    binding: mut current_value,
+                } = match KclValueGroup::from(first) {
+                    KclValueGroup::Single(v) => self.plan_to_compute_single(ctx, v)?,
+                    KclValueGroup::ArrayExpression(_) => todo!(),
+                    KclValueGroup::ObjectExpression(_) => todo!(),
+                };
+
+                // Handle the remaining bodies.
+                for body in bodies {
+                    let value = match KclValueGroup::from(body) {
+                        KclValueGroup::Single(v) => v,
+                        KclValueGroup::ArrayExpression(_) => todo!(),
+                        KclValueGroup::ObjectExpression(_) => todo!(),
+                    };
+                    // This body will probably contain a % (pipe substitution character).
+                    // So it needs to know what the previous pipeline body's value is,
+                    // to replace the % with that value.
+                    ctx.pipe_substitution = Some(current_value.clone());
+                    let EvalPlan {
+                        instructions: instructions_for_this_body,
+                        binding,
+                    } = self.plan_to_compute_single(ctx, value)?;
+                    instructions.extend(instructions_for_this_body);
+                    current_value = binding;
+                }
+                // Before we return, clear the pipe substitution, because nothing outside this
+                // pipeline should be able to use it anymore.
+                ctx.pipe_substitution = None;
+                Ok(EvalPlan {
+                    instructions,
+                    binding: current_value,
+                })
+            }
         }
     }
 
@@ -219,11 +406,12 @@ impl Planner {
         &mut self,
         declarations: ast::types::VariableDeclaration,
     ) -> Result<Vec<Instruction>, CompileError> {
+        let mut ctx = Context::default();
         declarations
             .declarations
             .into_iter()
             .try_fold(Vec::new(), |mut acc, declaration| {
-                let (instrs, binding) = self.plan_to_bind_one(declaration.init)?;
+                let (instrs, binding) = self.plan_to_bind_one(&mut ctx, declaration.init)?;
                 self.binding_scope.bind(declaration.id.name, binding);
                 acc.extend(instrs);
                 Ok(acc)
@@ -232,13 +420,14 @@ impl Planner {
 
     fn plan_to_bind_one(
         &mut self,
+        ctx: &mut Context,
         value_being_bound: ast::types::Value,
     ) -> Result<(Vec<Instruction>, EpBinding), CompileError> {
         match KclValueGroup::from(value_being_bound) {
             KclValueGroup::Single(init_value) => {
                 // Simple! Just evaluate it, note where the final value will be stored in KCEP memory,
                 // and bind it to the KCL identifier.
-                let EvalPlan { instructions, binding } = self.plan_to_compute_single(init_value)?;
+                let EvalPlan { instructions, binding } = self.plan_to_compute_single(ctx, init_value)?;
                 Ok((instructions, binding))
             }
             KclValueGroup::ArrayExpression(expr) => {
@@ -251,7 +440,7 @@ impl Planner {
                             KclValueGroup::Single(value) => {
                                 // If this element of the array is a single value, then binding it is
                                 // straightforward -- you got a single binding, no need to change anything.
-                                let EvalPlan { instructions, binding } = self.plan_to_compute_single(value)?;
+                                let EvalPlan { instructions, binding } = self.plan_to_compute_single(ctx, value)?;
                                 acc_instrs.extend(instructions);
                                 acc_bindings.push(binding);
                             }
@@ -264,7 +453,7 @@ impl Planner {
                                     .elements
                                     .into_iter()
                                     .try_fold(Vec::new(), |mut seq, child_element| {
-                                        let (instructions, binding) = self.plan_to_bind_one(child_element)?;
+                                        let (instructions, binding) = self.plan_to_bind_one(ctx, child_element)?;
                                         acc_instrs.extend(instructions);
                                         seq.push(binding);
                                         Ok(seq)
@@ -282,7 +471,7 @@ impl Planner {
                                     .properties
                                     .into_iter()
                                     .try_fold(map, |mut map, property| {
-                                        let (instructions, binding) = self.plan_to_bind_one(property.value)?;
+                                        let (instructions, binding) = self.plan_to_bind_one(ctx, property.value)?;
                                         map.insert(property.key.name, binding);
                                         acc_instrs.extend(instructions);
                                         Ok(map)
@@ -304,7 +493,7 @@ impl Planner {
                     |(mut acc_instrs, mut acc_bindings), (key, value)| {
                         match KclValueGroup::from(value) {
                             KclValueGroup::Single(value) => {
-                                let EvalPlan { instructions, binding } = self.plan_to_compute_single(value)?;
+                                let EvalPlan { instructions, binding } = self.plan_to_compute_single(ctx, value)?;
                                 acc_instrs.extend(instructions);
                                 acc_bindings.insert(key.name, binding);
                             }
@@ -317,7 +506,7 @@ impl Planner {
                                     .elements
                                     .into_iter()
                                     .try_fold(Vec::with_capacity(n), |mut seq, child_element| {
-                                        let (instructions, binding) = self.plan_to_bind_one(child_element)?;
+                                        let (instructions, binding) = self.plan_to_bind_one(ctx, child_element)?;
                                         seq.push(binding);
                                         acc_instrs.extend(instructions);
                                         Ok(seq)
@@ -335,7 +524,7 @@ impl Planner {
                                     .properties
                                     .into_iter()
                                     .try_fold(HashMap::with_capacity(n), |mut map, property| {
-                                        let (instructions, binding) = self.plan_to_bind_one(property.value)?;
+                                        let (instructions, binding) = self.plan_to_bind_one(ctx, property.value)?;
                                         map.insert(property.key.name, binding);
                                         acc_instrs.extend(instructions);
                                         Ok(map)
@@ -353,52 +542,6 @@ impl Planner {
     }
 }
 
-#[derive(Debug, thiserror::Error, PartialEq, Clone)]
-pub enum CompileError {
-    #[error("the name {name} was not defined")]
-    Undefined { name: String },
-    #[error("the function {fn_name} requires at least {required} arguments but you only supplied {actual}")]
-    NotEnoughArgs {
-        fn_name: String2,
-        required: usize,
-        actual: usize,
-    },
-    #[error("the function {fn_name} accepts at most {maximum} arguments but you supplied {actual}")]
-    TooManyArgs {
-        fn_name: String2,
-        maximum: usize,
-        actual: usize,
-    },
-    #[error("you tried to call {name} but it's not a function")]
-    NotCallable { name: String },
-    #[error("you're trying to use an operand that isn't compatible with the given arithmetic operator: {0}")]
-    InvalidOperand(&'static str),
-    #[error("you cannot use the value {0} as an index")]
-    InvalidIndex(String),
-    #[error("you tried to index into a value that isn't an array. Only arrays have numeric indices!")]
-    CannotIndex,
-    #[error("you tried to get the element {i} but that index is out of bounds. The array only has a length of {len}")]
-    IndexOutOfBounds { i: usize, len: usize },
-    #[error("you tried to access the property of a value that doesn't have any properties")]
-    NoProperties,
-    #[error("you tried to access a property of an array, but arrays don't have properties. They do have numeric indexes though, try using an index e.g. [0]")]
-    ArrayDoesNotHaveProperties,
-    #[error(
-        "you tried to read the '.{property}' of an object, but the object doesn't have any properties with that key"
-    )]
-    UndefinedProperty { property: String },
-}
-
-#[derive(Debug, thiserror::Error)]
-pub enum Error {
-    #[error("{0}")]
-    Compile(#[from] CompileError),
-    #[error("{0}")]
-    Execution(#[from] ExecutionError),
-}
-
-type PlanRes = Result<Vec<Instruction>, CompileError>;
-
 /// Every KCL literal value is equivalent to an Execution Plan value, and therefore can be
 /// bound to some KCL name and Execution Plan address.
 fn kcl_literal_to_kcep_literal(expr: LiteralValue) -> ept::Primitive {
@@ -417,9 +560,35 @@ struct EvalPlan {
     binding: EpBinding,
 }
 
-trait KclFunction: std::fmt::Debug {
-    fn call(&self, next_addr: &mut Address, args: Vec<EpBinding>) -> Result<EvalPlan, CompileError>;
-}
-
 /// Either an owned string, or a static string. Either way it can be read and moved around.
 pub type String2 = std::borrow::Cow<'static, str>;
+
+#[derive(Debug, Clone)]
+struct UserDefinedFunction {
+    params_optional: Vec<ast::types::Parameter>,
+    params_required: Vec<ast::types::Parameter>,
+    body: ast::types::Program,
+}
+
+impl PartialEq for UserDefinedFunction {
+    fn eq(&self, other: &Self) -> bool {
+        self.params_optional == other.params_optional && self.params_required == other.params_required
+    }
+}
+
+impl Eq for UserDefinedFunction {}
+
+#[derive(Debug, Clone)]
+#[cfg_attr(test, derive(Eq, PartialEq))]
+enum KclFunction {
+    Id(native_functions::Id),
+    StartSketchAt(native_functions::StartSketchAt),
+    Add(native_functions::Add),
+    UserDefined(UserDefinedFunction),
+}
+
+/// Context used when compiling KCL.
+#[derive(Default, Debug)]
+struct Context {
+    pipe_substitution: Option<EpBinding>,
+}

src/wasm-lib/grackle/src/native_functions.rs

@@ -3,16 +3,21 @@
 //! But some other stdlib functions will be written in KCL.
 
 use kcl_lib::std::sketch::PlaneData;
-use kittycad_execution_plan::{Address, Arithmetic, Instruction};
+use kittycad_execution_plan::{Address, BinaryArithmetic, Instruction};
 use kittycad_execution_plan_traits::Value;
 
-use crate::{CompileError, EpBinding, EvalPlan, KclFunction};
+use crate::{CompileError, EpBinding, EvalPlan};
 
 /// The identity function. Always returns its first input.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
+#[cfg_attr(test, derive(Eq, PartialEq))]
 pub struct Id;
 
-impl KclFunction for Id {
+pub trait Callable {
+    fn call(&self, next_addr: &mut Address, args: Vec<EpBinding>) -> Result<EvalPlan, CompileError>;
+}
+
+impl Callable for Id {
     fn call(&self, _: &mut Address, args: Vec<EpBinding>) -> Result<EvalPlan, CompileError> {
         if args.len() > 1 {
             return Err(CompileError::TooManyArgs {
@@ -36,10 +41,11 @@ impl KclFunction for Id {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
+#[cfg_attr(test, derive(Eq, PartialEq))]
 pub struct StartSketchAt;
 
-impl KclFunction for StartSketchAt {
+impl Callable for StartSketchAt {
     fn call(&self, next_addr: &mut Address, _args: Vec<EpBinding>) -> Result<EvalPlan, CompileError> {
         let mut instructions = Vec::new();
         // Store the plane.
@@ -64,10 +70,11 @@ impl KclFunction for StartSketchAt {
 }
 
 /// A test function that adds two numbers.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
+#[cfg_attr(test, derive(Eq, PartialEq))]
 pub struct Add;
 
-impl KclFunction for Add {
+impl Callable for Add {
     fn call(&self, next_address: &mut Address, mut args: Vec<EpBinding>) -> Result<EvalPlan, CompileError> {
         let len = args.len();
         if len > 2 {
@@ -91,9 +98,9 @@ impl KclFunction for Add {
         };
         let destination = next_address.offset_by(1);
         Ok(EvalPlan {
-            instructions: vec![Instruction::Arithmetic {
-                arithmetic: Arithmetic {
-                    operation: kittycad_execution_plan::Operation::Add,
+            instructions: vec![Instruction::BinaryArithmetic {
+                arithmetic: BinaryArithmetic {
+                    operation: kittycad_execution_plan::BinaryOperation::Add,
                     operand0: kittycad_execution_plan::Operand::Reference(arg0),
                     operand1: kittycad_execution_plan::Operand::Reference(arg1),
                 },

src/wasm-lib/grackle/src/tests.rs

@@ -1,3 +1,4 @@
+use ep::UnaryArithmetic;
 use pretty_assertions::assert_eq;
 
 use super::*;
@@ -7,7 +8,7 @@ fn must_plan(program: &str) -> (Vec<Instruction>, BindingScope) {
     let parser = kcl_lib::parser::Parser::new(tokens);
     let ast = parser.ast().unwrap();
     let mut p = Planner::new();
-    let instrs = p.build_plan(ast).unwrap();
+    let (instrs, _) = p.build_plan(ast).unwrap();
     (instrs, p.binding_scope)
 }
 
@@ -109,6 +110,17 @@ fn bind_arrays_with_objects_elements() {
     );
 }
 
+#[test]
+fn statement_after_return() {
+    let program = "fn f = () => {
+        return 1
+        let x = 2
+    }
+    f()";
+    let err = should_not_compile(program);
+    assert_eq!(err, CompileError::MultipleReturns);
+}
+
 #[test]
 fn name_not_found() {
     // Users can't assign `y` to anything because `y` is undefined.
@@ -116,6 +128,23 @@ fn name_not_found() {
     assert_eq!(err, CompileError::Undefined { name: "y".to_owned() });
 }
 
+#[test]
+fn assign_bool() {
+    // Check that Grackle properly compiles KCL bools to EP bools.
+    for (str, val) in [("true", true), ("false", false)] {
+        let program = format!("let x = {str}");
+        let (plan, scope) = must_plan(&program);
+        assert_eq!(
+            plan,
+            vec![Instruction::SetPrimitive {
+                address: Address::ZERO,
+                value: val.into(),
+            }]
+        );
+        assert_eq!(scope.get("x"), Some(&EpBinding::Single(Address::ZERO)));
+    }
+}
+
 #[test]
 fn aliases() {
     let program = "
@@ -146,9 +175,9 @@ fn use_native_function_add() {
                 address: Address::ZERO.offset(1),
                 value: 2i64.into()
             },
-            Instruction::Arithmetic {
-                arithmetic: ep::Arithmetic {
-                    operation: ep::Operation::Add,
+            Instruction::BinaryArithmetic {
+                arithmetic: ep::BinaryArithmetic {
+                    operation: ep::BinaryOperation::Add,
                     operand0: ep::Operand::Reference(Address::ZERO),
                     operand1: ep::Operand::Reference(Address::ZERO.offset(1))
                 },
@@ -252,6 +281,27 @@ fn member_expressions_array() {
     }
 }
 
+#[test]
+fn compile_flipped_sign() {
+    let program = "let x = 3
+    let y = -x";
+    let (plan, _scope) = must_plan(program);
+    let expected = vec![
+        Instruction::SetPrimitive {
+            address: Address::ZERO,
+            value: 3i64.into(),
+        },
+        Instruction::UnaryArithmetic {
+            arithmetic: UnaryArithmetic {
+                operation: ep::UnaryOperation::Neg,
+                operand: ep::Operand::Reference(Address::ZERO),
+            },
+            destination: Address::ZERO + 1,
+        },
+    ];
+    assert_eq!(plan, expected);
+}
+
 #[test]
 fn add_literals() {
     let program = "let x = 1 + 2";
@@ -267,9 +317,9 @@ fn add_literals() {
                 address: Address::ZERO.offset(1),
                 value: 2i64.into()
             },
-            Instruction::Arithmetic {
-                arithmetic: ep::Arithmetic {
-                    operation: ep::Operation::Add,
+            Instruction::BinaryArithmetic {
+                arithmetic: ep::BinaryArithmetic {
+                    operation: ep::BinaryOperation::Add,
                     operand0: ep::Operand::Reference(Address::ZERO),
                     operand1: ep::Operand::Reference(Address::ZERO.offset(1)),
                 },
@@ -299,9 +349,9 @@ fn add_vars() {
                 address: addr1,
                 value: 2i64.into(),
             },
-            Instruction::Arithmetic {
-                arithmetic: ep::Arithmetic {
-                    operation: ep::Operation::Add,
+            Instruction::BinaryArithmetic {
+                arithmetic: ep::BinaryArithmetic {
+                    operation: ep::BinaryOperation::Add,
                     operand0: ep::Operand::Reference(addr0),
                     operand1: ep::Operand::Reference(addr1),
                 },
@@ -340,18 +390,18 @@ fn composite_binary_exprs() {
                 value: 3i64.into(),
             },
             // Adds 1 + 2
-            Instruction::Arithmetic {
-                arithmetic: ep::Arithmetic {
-                    operation: ep::Operation::Add,
+            Instruction::BinaryArithmetic {
+                arithmetic: ep::BinaryArithmetic {
+                    operation: ep::BinaryOperation::Add,
                     operand0: ep::Operand::Reference(addr0),
                     operand1: ep::Operand::Reference(addr1),
                 },
                 destination: addr3,
             },
             // Adds `x` + 3, where `x` is (1 + 2)
-            Instruction::Arithmetic {
-                arithmetic: ep::Arithmetic {
-                    operation: ep::Operation::Add,
+            Instruction::BinaryArithmetic {
+                arithmetic: ep::BinaryArithmetic {
+                    operation: ep::BinaryOperation::Add,
                     operand0: ep::Operand::Reference(addr3),
                     operand1: ep::Operand::Reference(addr2),
                 },
@@ -361,6 +411,238 @@ fn composite_binary_exprs() {
     );
 }
 
+#[test]
+fn use_kcl_functions_zero_params() {
+    let (plan, scope) = must_plan(
+        "fn triple = () => { return 123 }
+    let x = triple()",
+    );
+    assert_eq!(
+        plan,
+        vec![Instruction::SetPrimitive {
+            address: Address::ZERO,
+            value: 123i64.into()
+        }]
+    );
+    match scope.get("x").unwrap() {
+        EpBinding::Single(addr) => {
+            assert_eq!(addr, &Address::ZERO);
+        }
+        other => {
+            panic!("expected 'x' bound to an address but it was bound to {other:?}");
+        }
+    }
+}
+
+#[test]
+fn use_kcl_functions_with_optional_params() {
+    for (i, program) in ["fn triple = (x, y?) => { return x*3 }
+    let x = triple(1, 888)"]
+    .into_iter()
+    .enumerate()
+    {
+        let (plan, scope) = must_plan(program);
+        let destination = Address::ZERO + 3;
+        assert_eq!(
+            plan,
+            vec![
+                Instruction::SetPrimitive {
+                    address: Address::ZERO,
+                    value: 1i64.into(),
+                },
+                Instruction::SetPrimitive {
+                    address: Address::ZERO + 1,
+                    value: 888i64.into(),
+                },
+                Instruction::SetPrimitive {
+                    address: Address::ZERO + 2,
+                    value: 3i64.into(),
+                },
+                Instruction::BinaryArithmetic {
+                    arithmetic: ep::BinaryArithmetic {
+                        operation: ep::BinaryOperation::Mul,
+                        operand0: ep::Operand::Reference(Address::ZERO),
+                        operand1: ep::Operand::Reference(Address::ZERO + 2)
+                    },
+                    destination,
+                }
+            ],
+            "failed test {i}"
+        );
+        match scope.get("x").unwrap() {
+            EpBinding::Single(addr) => {
+                assert_eq!(addr, &destination, "failed test {i}");
+            }
+            other => {
+                panic!("expected 'x' bound to an address but it was bound to {other:?}, so failed test {i}");
+            }
+        }
+    }
+}
+
+#[test]
+fn use_kcl_functions_with_too_many_params() {
+    let program = "fn triple = (x, y?) => { return x*3 }
+    let x = triple(1, 2, 3)";
+    let err = should_not_compile(program);
+    assert!(matches!(
+        err,
+        CompileError::TooManyArgs {
+            maximum: 2,
+            actual: 3,
+            ..
+        }
+    ))
+}
+
+#[test]
+fn use_kcl_function_as_return_value() {
+    let program = "fn twotwotwo = () => {
+            return () => { return 222 }
+        }
+        let f = twotwotwo()
+        let x = f()";
+    let (plan, scope) = must_plan(program);
+    match scope.get("x").unwrap() {
+        EpBinding::Single(addr) => {
+            assert_eq!(addr, &Address::ZERO);
+        }
+        other => {
+            panic!("expected 'x' bound to an address but it was bound to {other:?}, so failed test");
+        }
+    }
+    assert_eq!(
+        plan,
+        vec![Instruction::SetPrimitive {
+            address: Address::ZERO,
+            value: 222i64.into()
+        }]
+    )
+}
+
+#[test]
+fn define_recursive_function() {
+    let program = "fn add_infinitely = (i) => {
+            return add_infinitely(i+1)
+        }";
+    let (plan, _scope) = must_plan(program);
+    assert_eq!(plan, Vec::new())
+}
+#[test]
+fn use_kcl_function_as_param() {
+    let program = "fn wrapper = (f) => {
+            return f()
+        }
+        fn twotwotwo = () => { 
+            return 222
+        }
+        let x = wrapper(twotwotwo)";
+    let (plan, scope) = must_plan(program);
+    match scope.get("x").unwrap() {
+        EpBinding::Single(addr) => {
+            assert_eq!(addr, &Address::ZERO);
+        }
+        other => {
+            panic!("expected 'x' bound to an address but it was bound to {other:?}, so failed test");
+        }
+    }
+    assert_eq!(
+        plan,
+        vec![Instruction::SetPrimitive {
+            address: Address::ZERO,
+            value: 222i64.into()
+        }]
+    )
+}
+
+#[test]
+fn use_kcl_functions_with_params() {
+    for (i, program) in [
+        "fn triple = (x) => { return x*3 }
+    let x = triple(1)",
+        "fn triple = (x,y?) => { return x*3 }
+    let x = triple(1)",
+    ]
+    .into_iter()
+    .enumerate()
+    {
+        let (plan, scope) = must_plan(program);
+        let destination = Address::ZERO + 2;
+        assert_eq!(
+            plan,
+            vec![
+                Instruction::SetPrimitive {
+                    address: Address::ZERO,
+                    value: 1i64.into(),
+                },
+                Instruction::SetPrimitive {
+                    address: Address::ZERO + 1,
+                    value: 3i64.into(),
+                },
+                Instruction::BinaryArithmetic {
+                    arithmetic: ep::BinaryArithmetic {
+                        operation: ep::BinaryOperation::Mul,
+                        operand0: ep::Operand::Reference(Address::ZERO),
+                        operand1: ep::Operand::Reference(Address::ZERO.offset(1))
+                    },
+                    destination,
+                }
+            ],
+            "failed test {i}"
+        );
+        match scope.get("x").unwrap() {
+            EpBinding::Single(addr) => {
+                assert_eq!(addr, &destination, "failed test {i}");
+            }
+            other => {
+                panic!("expected 'x' bound to an address but it was bound to {other:?}, so failed test {i}");
+            }
+        }
+    }
+}
+
+#[test]
+fn pipe_substitution_outside_pipe_expression() {
+    let program = "let x = add(1, %)";
+    let err = should_not_compile(program);
+    assert!(matches!(err, CompileError::NotInPipeline));
+}
+
+#[test]
+fn unsugar_pipe_expressions() {
+    // These two programs should be equivalent,
+    // because that's just the definition of the |> operator.
+    let program2 = "
+    fn double = (x) => { return x * 2 }
+    fn triple = (x) => { return x * 3 }
+    let x = 1 |> double(%) |> triple(%) // should be 6
+    ";
+    let program1 = "
+    fn double = (x) => { return x * 2 }
+    fn triple = (x) => { return x * 3 }
+    let x = triple(double(1)) // should be 6
+    ";
+    // So, check that they are.
+    let (plan1, _) = must_plan(program1);
+    let (plan2, _) = must_plan(program2);
+    assert_eq!(plan1, plan2);
+}
+
+#[test]
+fn define_kcl_functions() {
+    let (plan, scope) = must_plan("fn triple = (x) => { return x * 3 }");
+    assert!(plan.is_empty());
+    match scope.get("triple").unwrap() {
+        EpBinding::Function(KclFunction::UserDefined(expr)) => {
+            assert!(expr.params_optional.is_empty());
+            assert_eq!(expr.params_required.len(), 1);
+        }
+        other => {
+            panic!("expected 'triple' bound to a user-defined KCL function but it was bound to {other:?}");
+        }
+    }
+}
+
 #[test]
 fn aliases_dont_affect_plans() {
     let (plan1, _) = must_plan(

src/wasm-lib/kcl/Cargo.toml

@@ -32,6 +32,7 @@ thiserror = "1.0.50"
 ts-rs = { version = "7", features = ["uuid-impl"] }
 uuid = { version = "1.6.1", features = ["v4", "js", "serde"] }
 winnow = "0.5.18"
+either = "1.6.1"
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 js-sys = { version = "0.3.65" }

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

@@ -19,6 +19,7 @@ use crate::{
     parser::PIPE_OPERATOR,
     std::{kcl_stdlib::KclStdLibFn, FunctionKind},
 };
+use crate::executor::PathToNode;
 
 mod literal_value;
 mod none;
@@ -1433,6 +1434,7 @@ impl From<Literal> for MemoryItem {
             value: JValue::from(literal.value.clone()),
             meta: vec![Metadata {
                 source_range: literal.into(),
+                path_to_node: vec![],
             }],
         })
     }
@@ -1444,6 +1446,7 @@ impl From<&Box<Literal>> for MemoryItem {
             value: JValue::from(literal.value.clone()),
             meta: vec![Metadata {
                 source_range: literal.into(),
+                path_to_node: vec![],
             }],
         })
     }
@@ -1641,7 +1644,7 @@ impl ArrayExpression {
                 Value::UnaryExpression(unary_expression) => unary_expression.get_result(memory, pipe_info, ctx).await?,
                 Value::ObjectExpression(object_expression) => object_expression.execute(memory, pipe_info, ctx).await?,
                 Value::ArrayExpression(array_expression) => array_expression.execute(memory, pipe_info, ctx).await?,
-                Value::PipeExpression(pipe_expression) => pipe_expression.get_result(memory, pipe_info, ctx).await?,
+                Value::PipeExpression(pipe_expression) => pipe_expression.get_result(memory, pipe_info, ctx, vec![]).await?,
                 Value::PipeSubstitution(pipe_substitution) => {
                     return Err(KclError::Semantic(KclErrorDetails {
                         message: format!("PipeSubstitution not implemented here: {:?}", pipe_substitution),
@@ -1665,6 +1668,7 @@ impl ArrayExpression {
             value: results.into(),
             meta: vec![Metadata {
                 source_range: self.into(),
+                path_to_node: vec![],
             }],
         }))
     }
@@ -1794,7 +1798,7 @@ impl ObjectExpression {
                 Value::UnaryExpression(unary_expression) => unary_expression.get_result(memory, pipe_info, ctx).await?,
                 Value::ObjectExpression(object_expression) => object_expression.execute(memory, pipe_info, ctx).await?,
                 Value::ArrayExpression(array_expression) => array_expression.execute(memory, pipe_info, ctx).await?,
-                Value::PipeExpression(pipe_expression) => pipe_expression.get_result(memory, pipe_info, ctx).await?,
+                Value::PipeExpression(pipe_expression) => pipe_expression.get_result(memory, pipe_info, ctx, vec![]).await?,
                 Value::PipeSubstitution(pipe_substitution) => {
                     return Err(KclError::Semantic(KclErrorDetails {
                         message: format!("PipeSubstitution not implemented here: {:?}", pipe_substitution),
@@ -1822,6 +1826,7 @@ impl ObjectExpression {
             value: object.into(),
             meta: vec![Metadata {
                 source_range: self.into(),
+                path_to_node: vec![],
             }],
         }))
     }
@@ -2031,6 +2036,7 @@ impl MemberExpression {
                     value: value.clone(),
                     meta: vec![Metadata {
                         source_range: self.into(),
+                        path_to_node: vec![],
                     }],
                 }))
             } else {
@@ -2087,6 +2093,7 @@ impl MemberExpression {
                     value: value.clone(),
                     meta: vec![Metadata {
                         source_range: self.into(),
+                        path_to_node: vec![],
                     }],
                 }))
             } else {
@@ -2251,6 +2258,7 @@ impl BinaryExpression {
                     value,
                     meta: vec![Metadata {
                         source_range: self.into(),
+                        path_to_node: vec![],
                     }],
                 }));
             }
@@ -2272,6 +2280,7 @@ impl BinaryExpression {
             value,
             meta: vec![Metadata {
                 source_range: self.into(),
+                path_to_node: vec![],
             }],
         }))
     }
@@ -2435,6 +2444,7 @@ impl UnaryExpression {
             value: (-(num)).into(),
             meta: vec![Metadata {
                 source_range: self.into(),
+                path_to_node: vec![],
             }],
         }))
     }
@@ -2564,11 +2574,12 @@ impl PipeExpression {
         memory: &mut ProgramMemory,
         pipe_info: &mut PipeInfo,
         ctx: &ExecutorContext,
+        path_to_node: PathToNode,
     ) -> Result<MemoryItem, KclError> {
         // Reset the previous results.
         pipe_info.previous_results = vec![];
         pipe_info.index = 0;
-        execute_pipe_body(memory, &self.body, pipe_info, self.into(), ctx).await
+        execute_pipe_body(memory, &self.body, pipe_info, self.into(), ctx, path_to_node).await
     }
 
     /// Rename all identifiers that have the old name to the new given name.
@@ -2586,6 +2597,8 @@ async fn execute_pipe_body(
     pipe_info: &mut PipeInfo,
     source_range: SourceRange,
     ctx: &ExecutorContext,
+    path_to_node: PathToNode,
+    
 ) -> Result<MemoryItem, KclError> {
     if pipe_info.index == body.len() {
         pipe_info.is_in_pipe = false;
@@ -2631,7 +2644,7 @@ async fn execute_pipe_body(
 }
 
 /// Parameter of a KCL function.
-#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema, Bake)]
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, Eq, ts_rs::TS, JsonSchema, Bake)]
 #[databake(path = kcl_lib::ast::types)]
 #[ts(export)]
 #[serde(tag = "type")]
@@ -2655,6 +2668,23 @@ pub struct FunctionExpression {
 
 impl_value_meta!(FunctionExpression);
 
+pub struct FunctionExpressionParts {
+    pub start: usize,
+    pub end: usize,
+    pub params_required: Vec<Parameter>,
+    pub params_optional: Vec<Parameter>,
+    pub body: Program,
+}
+
+#[derive(Debug, PartialEq, Clone)]
+pub struct RequiredParamAfterOptionalParam(pub Parameter);
+
+impl std::fmt::Display for RequiredParamAfterOptionalParam {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "KCL functions must declare any optional parameters after all the required parameters. But your required parameter {} is _after_ an optional parameter. You must move it to before the optional parameters instead.", self.0.identifier.name)
+    }
+}
+
 impl FunctionExpression {
     /// Function expressions don't really apply.
     pub fn get_constraint_level(&self) -> ConstraintLevel {
@@ -2663,6 +2693,36 @@ impl FunctionExpression {
         }
     }
 
+    pub fn into_parts(self) -> Result<FunctionExpressionParts, RequiredParamAfterOptionalParam> {
+        let Self {
+            start,
+            end,
+            params,
+            body,
+        } = self;
+        let mut params_required = Vec::with_capacity(params.len());
+        let mut params_optional = Vec::with_capacity(params.len());
+        for param in params {
+            if param.optional {
+                params_optional.push(param);
+            } else {
+                if !params_optional.is_empty() {
+                    return Err(RequiredParamAfterOptionalParam(param));
+                }
+                params_required.push(param);
+            }
+        }
+        params_required.shrink_to_fit();
+        params_optional.shrink_to_fit();
+        Ok(FunctionExpressionParts {
+            start,
+            end,
+            params_required,
+            params_optional,
+            body,
+        })
+    }
+
     /// Required parameters must be declared before optional parameters.
     /// This gets all the required parameters.
     pub fn required_params(&self) -> &[Parameter] {

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

@@ -11,6 +11,7 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use serde_json::Value as JValue;
 use tower_lsp::lsp_types::{Position as LspPosition, Range as LspRange};
+// use either::Either;
 
 use crate::{
     ast::types::{BodyItem, FunctionExpression, KclNone, Value},
@@ -632,6 +633,18 @@ impl From<Point3d> for kittycad::types::Point3D {
     }
 }
 
+/// number or string
+#[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
+#[ts(export)]
+pub enum NumberOrString {
+    Num(i32), // assuming 'number' is equivalent to a 32-bit integer
+    Str(String),
+}
+
+/// PathToNode
+pub type PathToNode = Vec<(NumberOrString, String)>;
+
+
 /// Metadata.
 #[derive(Debug, Clone, Deserialize, Serialize, PartialEq, ts_rs::TS, JsonSchema)]
 #[ts(export)]
@@ -639,11 +652,16 @@ impl From<Point3d> for kittycad::types::Point3D {
 pub struct Metadata {
     /// The source range.
     pub source_range: SourceRange,
+    /// The path to node for this memory Item
+    pub path_to_node: PathToNode,
 }
 
 impl From<SourceRange> for Metadata {
     fn from(source_range: SourceRange) -> Self {
-        Self { source_range }
+        Self {
+            source_range,
+            path_to_node: Vec::new()
+        }
     }
 }
 
@@ -829,12 +847,17 @@ pub async fn execute(
 ) -> Result<ProgramMemory, KclError> {
     let mut pipe_info = PipeInfo::default();
 
+    // let path_to_Node: PathToNode = vec![("body".to_string(), "".to_string())];
+    let path_to_node: PathToNode = vec![(NumberOrString::Str("body".to_string()), "".to_string())];
+
     // Iterate over the body of the program.
-    for statement in &program.body {
+    for (index, statement) in program.body.iter().enumerate() {
+        let mut with_body_path_to_node = path_to_node.clone();
+        with_body_path_to_node.push((NumberOrString::Num(index as i32), "index".to_string()));
         match statement {
             BodyItem::ExpressionStatement(expression_statement) => {
                 if let Value::PipeExpression(pipe_expr) = &expression_statement.expression {
-                    pipe_expr.get_result(memory, &mut pipe_info, ctx).await?;
+                    pipe_expr.get_result(memory, &mut pipe_info, ctx, with_body_path_to_node).await?;
                 } else if let Value::CallExpression(call_expr) = &expression_statement.expression {
                     let fn_name = call_expr.callee.name.to_string();
                     let mut args: Vec<MemoryItem> = Vec::new();
@@ -905,10 +928,15 @@ pub async fn execute(
                 }
             }
             BodyItem::VariableDeclaration(variable_declaration) => {
-                for declaration in &variable_declaration.declarations {
+                
+                for (index, declaration) in variable_declaration.declarations.iter().enumerate() {
                     let var_name = declaration.id.name.to_string();
                     let source_range: SourceRange = declaration.init.clone().into();
-                    let metadata = Metadata { source_range };
+                    let mut with_dec_path_to_node = with_body_path_to_node.clone();
+                    with_dec_path_to_node.push((NumberOrString::Str("declarations".to_string()), "VariableDeclaration".to_string()));
+                    with_dec_path_to_node.push((NumberOrString::Num(index as i32), "index".to_string()));
+                    with_dec_path_to_node.push((NumberOrString::Str("init".to_string()), "".to_string()));
+                    let metadata = Metadata { source_range, path_to_node: with_dec_path_to_node.clone() };
 
                     match &declaration.init {
                         Value::None(none) => {
@@ -963,7 +991,7 @@ pub async fn execute(
                             memory.add(&var_name, result, source_range)?;
                         }
                         Value::PipeExpression(pipe_expression) => {
-                            let result = pipe_expression.get_result(memory, &mut pipe_info, ctx).await?;
+                            let result = pipe_expression.get_result(memory, &mut pipe_info, ctx, with_dec_path_to_node).await?;
                             memory.add(&var_name, result, source_range)?;
                         }
                         Value::PipeSubstitution(pipe_substitution) => {
@@ -1027,7 +1055,7 @@ pub async fn execute(
                     memory.return_ = Some(ProgramReturn::Value(result));
                 }
                 Value::PipeExpression(pipe_expr) => {
-                    let result = pipe_expr.get_result(memory, &mut pipe_info, ctx).await?;
+                    let result = pipe_expr.get_result(memory, &mut pipe_info, ctx, with_body_path_to_node).await?;
                     memory.return_ = Some(ProgramReturn::Value(result));
                 }
                 Value::PipeSubstitution(_) => {}

src/wasm-lib/kcl/src/std/mod.rs

@@ -190,6 +190,7 @@ impl Args {
             value: j,
             meta: vec![Metadata {
                 source_range: self.source_range,
+                path_to_node: Vec::new()
             }],
         }))
     }

yarn.lock

@@ -8188,10 +8188,10 @@ vite-tsconfig-paths@^4.2.1:
     globrex "^0.1.2"
     tsconfck "^2.1.0"
 
-"vite@^3.0.0 || ^4.0.0 || ^5.0.0-0", "vite@^3.1.0 || ^4.0.0 || ^5.0.0-0", vite@^4.5.1:
-  version "4.5.1"
-  resolved "https://registry.yarnpkg.com/vite/-/vite-4.5.1.tgz#3370986e1ed5dbabbf35a6c2e1fb1e18555b968a"
-  integrity sha512-AXXFaAJ8yebyqzoNB9fu2pHoo/nWX+xZlaRwoeYUxEqBO+Zj4msE5G+BhGBll9lYEKv9Hfks52PAF2X7qDYXQA==
+"vite@^3.0.0 || ^4.0.0 || ^5.0.0-0", "vite@^3.1.0 || ^4.0.0 || ^5.0.0-0", vite@^4.5.2:
+  version "4.5.2"
+  resolved "https://registry.yarnpkg.com/vite/-/vite-4.5.2.tgz#d6ea8610e099851dad8c7371599969e0f8b97e82"
+  integrity sha512-tBCZBNSBbHQkaGyhGCDUGqeo2ph8Fstyp6FMSvTtsXeZSPpSMGlviAOav2hxVTqFcx8Hj/twtWKsMJXNY0xI8w==
   dependencies:
     esbuild "^0.18.10"
     postcss "^8.4.27"