Sketch mode more tolerant to syntax errors (#4056)

* add fix

* add test

* typos

* clean up

e2e/playwright/fixtures/editorFixture.ts

@@ -72,7 +72,7 @@ export class EditorFixture {
       const content = await this.diagnosticsTooltip.allTextContents()
       diagnosticsContent.push(content.join(''))
     }
-    return [...new Set(diagnosticsContent)].map((d) => d.trim())
+    return [...new Set(diagnosticsContent)].map((d) => sansWhitespace(d))
   }
 
   private _getHighlightedCode = async () => {
@@ -108,4 +108,11 @@ export class EditorFixture {
         diagnostics: expectedState.diagnostics.map(sansWhitespace),
       })
   }
+  replaceCode = async (findCode: string, replaceCode: string) => {
+    const lines = await this.page.locator('.cm-line').all()
+    let code = (await Promise.all(lines.map((c) => c.textContent()))).join('\n')
+    if (!lines) return
+    code = code.replace(findCode, replaceCode)
+    await this.codeContent.fill(code)
+  }
 }

e2e/playwright/fixtures/sceneFixture.ts

@@ -4,6 +4,7 @@ import { uuidv4 } from 'lib/utils'
 import {
   closeDebugPanel,
   doAndWaitForImageDiff,
+  getPixelRGBs,
   openAndClearDebugPanel,
   sendCustomCmd,
 } from '../test-utils'
@@ -89,4 +90,28 @@ export class SceneFixture {
   waitForExecutionDone = async () => {
     await expect(this.exeIndicator).toBeVisible()
   }
+
+  expectPixelColor = async (
+    colour: [number, number, number],
+    coords: { x: number; y: number },
+    diff: number
+  ) => {
+    let finalValue = colour
+    await expect
+      .poll(async () => {
+        const pixel = (await getPixelRGBs(this.page)(coords, 1))[0]
+        if (!pixel) return null
+        finalValue = pixel
+        return pixel.every(
+          (channel, index) => Math.abs(channel - colour[index]) < diff
+        )
+      })
+      .toBeTruthy()
+      .catch((cause) => {
+        throw new Error(
+          `ExpectPixelColor: expecting ${colour} got ${finalValue}`,
+          { cause }
+        )
+      })
+  }
 }

e2e/playwright/sketch-tests.spec.ts

@@ -1,4 +1,5 @@
 import { test, expect, Page } from '@playwright/test'
+import { test as test2, expect as expect2 } from './fixtures/fixtureSetup'
 
 import {
   getMovementUtils,
@@ -1107,3 +1108,64 @@ const sketch002 = startSketchOn(extrude001, 'END')
     }).toPass({ timeout: 40_000, intervals: [1_000] })
   })
 })
+
+test2.describe('Sketch mode should be toleratant to syntax errors', () => {
+  test2(
+    'adding a syntax error, recovers after fixing',
+    { tag: ['@skipWin'] },
+    async ({ app, scene, editor, toolbar }) => {
+      test.skip(
+        process.platform === 'win32',
+        'a codemirror error appears in this test only on windows, that causes the test to fail only because of our "no new error" logic, but it can not be replicated locally'
+      )
+      const file = await app.getInputFile('e2e-can-sketch-on-chamfer.kcl')
+      await app.initialise(file)
+
+      const [objClick] = scene.makeMouseHelpers(600, 250)
+      const arrowHeadLocation = { x: 604, y: 129 } as const
+      const arrowHeadWhite: [number, number, number] = [255, 255, 255]
+      const backgroundGray: [number, number, number] = [28, 28, 28]
+      const verifyArrowHeadColor = async (c: [number, number, number]) =>
+        scene.expectPixelColor(c, arrowHeadLocation, 15)
+
+      await test.step('check chamfer selection changes cursor positon', async () => {
+        await expect2(async () => {
+          // sometimes initial click doesn't register
+          await objClick()
+          await editor.expectActiveLinesToBe([
+            '|> startProfileAt([75.8, 317.2], %) // [$startCapTag, $EndCapTag]',
+          ])
+        }).toPass({ timeout: 15_000, intervals: [500] })
+      })
+
+      await test.step('enter sketch and sanity check segments have been drawn', async () => {
+        await toolbar.editSketch()
+        // this checks sketch segments have been drawn
+        await verifyArrowHeadColor(arrowHeadWhite)
+      })
+
+      await test.step('Make typo and check the segments have Disappeared and there is a syntax error', async () => {
+        await editor.replaceCode('lineTo([pro', 'badBadBadFn([pro')
+        await editor.expectState({
+          activeLines: [],
+          diagnostics: ['memoryitemkey`badBadBadFn`isnotdefined'],
+          highlightedCode: '',
+        })
+        // this checks sketch segments have failed to be drawn
+        await verifyArrowHeadColor(backgroundGray)
+      })
+
+      await test.step('', async () => {
+        await editor.replaceCode('badBadBadFn([pro', 'lineTo([pro')
+        await editor.expectState({
+          activeLines: [],
+          diagnostics: [],
+          highlightedCode: '',
+        })
+        // this checks sketch segments have been drawn
+        await verifyArrowHeadColor(arrowHeadWhite)
+      })
+      await app.page.waitForTimeout(100)
+    }
+  )
+})

e2e/playwright/test-utils.ts

@@ -438,34 +438,7 @@ export async function getUtils(page: Page, test_?: typeof test) {
       }
       return maxDiff
     },
-    getPixelRGBs: async (
-      coords: { x: number; y: number },
-      radius: number
-    ): Promise<[number, number, number][]> => {
-      const buffer = await page.screenshot({
-        fullPage: true,
-      })
-      const screenshot = await PNG.sync.read(buffer)
-      const pixMultiplier: number = await page.evaluate(
-        'window.devicePixelRatio'
-      )
-      const allCords: [number, number][] = [[coords.x, coords.y]]
-      for (let i = 1; i < radius; i++) {
-        allCords.push([coords.x + i, coords.y])
-        allCords.push([coords.x - i, coords.y])
-        allCords.push([coords.x, coords.y + i])
-        allCords.push([coords.x, coords.y - i])
-      }
-      return allCords.map(([x, y]) => {
-        const index =
-          (screenshot.width * y * pixMultiplier + x * pixMultiplier) * 4 // rbga is 4 channels
-        return [
-          screenshot.data[index],
-          screenshot.data[index + 1],
-          screenshot.data[index + 2],
-        ]
-      })
-    },
+    getPixelRGBs: getPixelRGBs(page),
     doAndWaitForImageDiff: (fn: () => Promise<unknown>, diffCount = 200) =>
       doAndWaitForImageDiff(page, fn, diffCount),
     emulateNetworkConditions: async (
@@ -1070,3 +1043,32 @@ export async function openAndClearDebugPanel(page: Page) {
 export function sansWhitespace(str: string) {
   return str.replace(/\s+/g, '').trim()
 }
+
+export function getPixelRGBs(page: Page) {
+  return async (
+    coords: { x: number; y: number },
+    radius: number
+  ): Promise<[number, number, number][]> => {
+    const buffer = await page.screenshot({
+      fullPage: true,
+    })
+    const screenshot = await PNG.sync.read(buffer)
+    const pixMultiplier: number = await page.evaluate('window.devicePixelRatio')
+    const allCords: [number, number][] = [[coords.x, coords.y]]
+    for (let i = 1; i < radius; i++) {
+      allCords.push([coords.x + i, coords.y])
+      allCords.push([coords.x - i, coords.y])
+      allCords.push([coords.x, coords.y + i])
+      allCords.push([coords.x, coords.y - i])
+    }
+    return allCords.map(([x, y]) => {
+      const index =
+        (screenshot.width * y * pixMultiplier + x * pixMultiplier) * 4 // rbga is 4 channels
+      return [
+        screenshot.data[index],
+        screenshot.data[index + 1],
+        screenshot.data[index + 2],
+      ]
+    })
+  }
+}

src/clientSideScene/sceneEntities.ts

@@ -1175,7 +1175,7 @@ export class SceneEntities {
     prepareTruncatedMemoryAndAst(
       sketchPathToNode,
       ast || kclManager.ast,
-      kclManager.programMemory,
+      kclManager.lastSuccessfulProgramMemory,
       draftSegment
     )
   onDragSegment({

src/lang/KclSingleton.ts

@@ -43,6 +43,7 @@ export class KclManager {
     digest: null,
   }
   private _programMemory: ProgramMemory = ProgramMemory.empty()
+  lastSuccessfulProgramMemory: ProgramMemory = ProgramMemory.empty()
   private _logs: string[] = []
   private _lints: Diagnostic[] = []
   private _kclErrors: KCLError[] = []
@@ -297,6 +298,9 @@ export class KclManager {
     // Do not add the errors since the program was interrupted and the error is not a real KCL error
     this.addKclErrors(isInterrupted ? [] : errors)
     this.programMemory = programMemory
+    if (!errors.length) {
+      this.lastSuccessfulProgramMemory = programMemory
+    }
     this.ast = { ...ast }
     this._executeCallback()
     this.engineCommandManager.addCommandLog({
@@ -342,6 +346,9 @@ export class KclManager {
     this._logs = logs
     this._kclErrors = errors
     this._programMemory = programMemory
+    if (!errors.length) {
+      this.lastSuccessfulProgramMemory = programMemory
+    }
     if (updates !== 'artifactRanges') return
 
     // TODO the below seems like a work around, I wish there's a comment explaining exactly what

src/lib/constants.ts

@@ -99,6 +99,6 @@ export const MAKE_TOAST_MESSAGES = {
 /** The URL for the KCL samples manifest files */
 export const KCL_SAMPLES_MANIFEST_URLS = {
   remote:
-    'https://raw.githubusercontent.com/KittyCAD/kcl-samples/main/manifst.json',
+    'https://raw.githubusercontent.com/KittyCAD/kcl-samples/main/manifest.json',
   localFallback: '/kcl-samples-manifest-fallback.json',
 } as const