More consistent error handling in modelingMachine codemods (#6910)

* First pass at consistency in modelingMachine codemods

* Add 'no kcl errors' guard instead of if check for all the existing ones

* Add more commands and improve consistency

* Add comments

* Fix test with old kcl that was showcasing the very behavior we're trying to fix
https://kittycadworkspace.slack.com/archives/C07A80B83FS/p1747231832870739?thread_ts=1747231178.515289&cid=C07A80B83FS

* Add test for sketch and helix

* Remove guard use and move hasErrors check closer to updateAst calls

* Revert "Remove guard use and move hasErrors check closer to updateAst calls"

This reverts commit 868ea4b605.

* Remove toasts from guards

* Remove some scene.settled calls

* Lint

* More shaky fixes

* Clean up and more test fixes

---------

Co-authored-by: Frank Noirot <frankjohnson1993@gmail.com>

e2e/playwright/code-pane-and-errors.spec.ts

@@ -134,8 +134,6 @@ extrude001 = extrude(sketch001, length = 5)`
     await page.setBodyDimensions({ width: 1200, height: 500 })
     await homePage.goToModelingScene()
 
-    await page.waitForTimeout(1000)
-
     // Ensure badge is present
     const codePaneButtonHolder = page.locator('#code-button-holder')
     await expect(codePaneButtonHolder).toContainText('notification', {
@@ -183,7 +181,7 @@ extrude001 = extrude(sketch001, length = 5)`
     await page.setBodyDimensions({ width: 1200, height: 500 })
     await homePage.goToModelingScene()
 
-    await scene.settled(cmdBar)
+    // await scene.settled(cmdBar)
 
     // Ensure badge is present
     const codePaneButtonHolder = page.locator('#code-button-holder')

e2e/playwright/editor-tests.spec.ts

@@ -1533,7 +1533,6 @@ sketch001 = startSketchOn(XZ)
     await homePage.goToModelingScene()
 
     await scene.connectionEstablished()
-    await scene.settled(cmdBar)
 
     await scene.expectPixelColor(
       TEST_COLORS.DARK_MODE_BKGD,

e2e/playwright/point-click.spec.ts

@@ -4943,4 +4943,34 @@ path001 = startProfile(sketch001, at = [0, 0])
       )
     })
   })
+
+  test(`Point and click codemods can't run on KCL errors`, async ({
+    context,
+    page,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    const badCode = `sketch001 = startSketchOn(XZ)
+profile001 = circle(sketch001, center = [0, 0], radius = 1)
+extrude001 = extrude(profile001 length = 1)`
+    await context.addInitScript((initialCode) => {
+      localStorage.setItem('persistCode', initialCode)
+    }, badCode)
+    await page.setBodyDimensions({ width: 1000, height: 500 })
+    await homePage.goToModelingScene()
+    await scene.connectionEstablished()
+
+    await test.step(`Start Sketch is disabled`, async () => {
+      await expect(toolbar.startSketchBtn).not.toBeEnabled()
+      await editor.expectEditor.toContain(badCode, { shouldNormalise: true })
+    })
+
+    await test.step(`Helix is disabled`, async () => {
+      await expect(toolbar.helixButton).not.toBeEnabled()
+      await editor.expectEditor.toContain(badCode, { shouldNormalise: true })
+    })
+  })
 })

e2e/playwright/regression-tests.spec.ts

@@ -19,11 +19,12 @@ test.describe('Regression tests', () => {
     context,
     page,
     homePage,
+    scene,
   }) => {
     // because the model has `line([0,0]..` it is valid code, but the model is invalid
     // regression test for https://github.com/KittyCAD/modeling-app/issues/3251
     // Since the bad model also found as issue with the artifact graph, which in tern blocked the editor diognostics
-    const u = await getUtils(page)
+    // const u = await getUtils(page)
     await context.addInitScript(async () => {
       localStorage.setItem(
         'persistCode',
@@ -40,7 +41,8 @@ test.describe('Regression tests', () => {
     await page.setBodyDimensions({ width: 1000, height: 500 })
 
     await homePage.goToModelingScene()
-    await u.waitForPageLoad()
+    await scene.connectionEstablished()
+    // await u.waitForPageLoad()
 
     // error in guter
     await expect(page.locator('.cm-lint-marker-error')).toBeVisible()
@@ -188,8 +190,8 @@ extrude001 = extrude(sketch001, length = 50)
       page.locator('.pretty-json-container >> text=myVar:"67')
     ).toBeVisible()
   })
-  test('ProgramMemory can be serialised', async ({ page, homePage }) => {
+  test('ProgramMemory can be serialised', async ({ page, homePage, scene }) => {
-    const u = await getUtils(page)
+    // const u = await getUtils(page)
     await page.addInitScript(async () => {
       localStorage.setItem(
         'persistCode',
@@ -214,11 +216,12 @@ extrude001 = extrude(sketch001, length = 50)
     // Listen for all console events and push the message text to an array
     page.on('console', (message) => messages.push(message.text()))
     await homePage.goToModelingScene()
-    await u.waitForPageLoad()
+    // await u.waitForPageLoad()
+    await scene.connectionEstablished()
 
     // wait for execution done
-    await u.openDebugPanel()
+    // await u.openDebugPanel()
-    await u.expectCmdLog('[data-message-type="execution-done"]')
+    // await u.expectCmdLog('[data-message-type="execution-done"]')
 
     const forbiddenMessages = ['cannot serialize tagged newtype variant']
     forbiddenMessages.forEach((forbiddenMessage) => {
@@ -232,6 +235,7 @@ extrude001 = extrude(sketch001, length = 50)
     context,
     page,
     homePage,
+    scene,
   }) => {
     const u = await getUtils(page)
     // const PUR = 400 / 37.5 //pixeltoUnitRatio
@@ -250,11 +254,10 @@ extrude001 = extrude(sketch001, length = 50)
     shell(exampleSketch, faces = ['end'], thickness = 0.25)`
       )
     })
+    await homePage.goToModelingScene()
+    await scene.connectionEstablished()
 
     await expect(async () => {
-      await homePage.goToModelingScene()
-      await u.waitForPageLoad()
-
       // error in guter
       await expect(page.locator('.cm-lint-marker-error')).toBeVisible({
         timeout: 1_000,

e2e/playwright/sketch-tests.spec.ts

@@ -1365,18 +1365,18 @@ solid001 = subtract([extrude001], tools = [extrude002])
     await page.addInitScript(async () => {
       localStorage.setItem(
         'persistCode',
-        `fn in2mm = (inches) => {
+        `fn in2mm(@inches) {
     return inches * 25.4
   }
 
-  const railTop = in2mm(.748)
+  railTop = in2mm(.748)
-  const railSide = in2mm(.024)
+  railSide = in2mm(.024)
-  const railBaseWidth = in2mm(.612)
+  railBaseWidth = in2mm(.612)
-  const railWideWidth = in2mm(.835)
+  railWideWidth = in2mm(.835)
-  const railBaseLength = in2mm(.200)
+  railBaseLength = in2mm(.200)
-  const railClampable = in2mm(.200)
+  railClampable = in2mm(.200)
 
-  const rail = startSketchOn(XZ)
+  rail = startSketchOn(XZ)
     |> startProfile(at = [-railTop / 2, railClampable + railBaseLength])
     |> line(endAbsolute = [
      railTop / 2,
@@ -3540,7 +3540,6 @@ profile001 = startProfile(sketch001, at = [127.56, 179.02])
 
     await homePage.openProject('multi-file-sketch-test')
     await scene.connectionEstablished()
-    await scene.settled(cmdBar)
 
     await u.closeDebugPanel()
 
@@ -3555,9 +3554,6 @@ profile001 = startProfile(sketch001, at = [127.56, 179.02])
 
     await toolbar.openFile('error.kcl')
 
-    // Ensure filetree is populated
-    await scene.settled(cmdBar)
-
     await expect(
       toolbar.featureTreePane.getByRole('button', { name: 'Sketch' })
     ).toHaveCount(0)

src/Toolbar.tsx

@@ -158,7 +158,8 @@ export function Toolbar({
       const isDisabled =
         disableAllButtons ||
         !isConfiguredAvailable ||
-        maybeIconConfig.disabled?.(state) === true
+        maybeIconConfig.disabled?.(state) === true ||
+        kclManager.hasErrors()
 
       return {
         ...maybeIconConfig,
@@ -444,6 +445,15 @@ const ToolbarItemTooltip = memo(function ToolbarItemContents({
       contentClassName={contentClassName}
     >
       {children}
+      {kclManager.hasErrors() && (
+        <p className="text-xs p-1 text-chalkboard-70 dark:text-chalkboard-40">
+          <CustomIcon
+            name="exclamationMark"
+            className="w-4 h-4 inline-block mr-1 text-destroy-80 bg-destroy-10"
+          />
+          Fix KCL errors to enable tools
+        </p>
+      )}
     </Tooltip>
   )
 })

src/components/CommandComboBox.tsx

@@ -136,8 +136,9 @@ function optionIsDisabled(option: Command): boolean {
     option.disabled ||
     ('machineActor' in option &&
       option.machineActor !== undefined &&
-      !getActorNextEvents(option.machineActor.getSnapshot()).includes(
+      (!getActorNextEvents(option.machineActor.getSnapshot()).includes(
         option.name
-      ))
+      ) ||
+        !option.machineActor?.getSnapshot().can({ type: option.name })))
   )
 }

src/components/ModelingMachineProvider.tsx

@@ -580,24 +580,23 @@ export const ModelingMachineProvider = ({
             selectionRanges
           )
         },
-        'Has exportable geometry': () => {
+        'Has exportable geometry': () =>
-          if (!kclManager.hasErrors() && kclManager.ast.body.length > 0)
+          !kclManager.hasErrors() && kclManager.ast.body.length > 0,
-            return true
-          else {
-            let errorMessage = 'Unable to Export '
-            if (kclManager.hasErrors()) errorMessage += 'due to KCL Errors'
-            else if (kclManager.ast.body.length === 0)
-              errorMessage += 'due to Empty Scene'
-            console.error(errorMessage)
-            toast.error(errorMessage)
-            return false
-          }
-        },
       },
       actors: {
         exportFromEngine: fromPromise(
           async ({ input }: { input?: ModelingCommandSchema['Export'] }) => {
-            if (!input) {
+            if (kclManager.hasErrors() || kclManager.ast.body.length === 0) {
+              let errorMessage = 'Unable to Export '
+              if (kclManager.hasErrors()) {
+                errorMessage += 'due to KCL Errors'
+              } else if (kclManager.ast.body.length === 0) {
+                errorMessage += 'due to Empty Scene'
+              }
+              console.error(errorMessage)
+              toast.error(errorMessage)
+              return new Error(errorMessage)
+            } else if (!input) {
               return new Error('No input provided')
             }