Extend point-and-click edit flow to non-pipe Chamfer and Fillet (#6767)

* enable non-piped fillets and chamfers

* reorder chamferAstMod

* tsc

* editEdgeTreatment + refactor + hookup

* remove unused stuff

* test

* typos

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

* else else else

* Apply suggestions from code review

pierre edits

Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com>

* const

* parameterName

* fmt

* efficiency !

* graphite being helpful

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>

---------

Co-authored-by: graphite-app[bot] <96075541+graphite-app[bot]@users.noreply.github.com>
Co-authored-by: Pierre Jacquier <pierrejacquier39@gmail.com>

e2e/playwright/point-click.spec.ts

@@ -2321,11 +2321,12 @@ extrude001 = extrude(sketch001, length = -12)
     })
   })
 
-  test(`Fillet point-and-click edit rejected when not in pipe`, async ({
+  test(`Fillet point-and-click edit standalone expression`, async ({
     context,
     page,
     homePage,
     scene,
+    editor,
     toolbar,
     cmdBar,
   }) => {
@@ -2339,23 +2340,44 @@ profile001 = circle(
 extrude001 = extrude(profile001, length = 100)
 fillet001 = fillet(extrude001, radius = 5, tags = [getOppositeEdge(seg01)])
 `
-    await context.addInitScript((initialCode) => {
+    await test.step(`Initial test setup`, async () => {
-      localStorage.setItem('persistCode', initialCode)
+      await context.addInitScript((initialCode) => {
-    }, initialCode)
+        localStorage.setItem('persistCode', initialCode)
-    await page.setBodyDimensions({ width: 1000, height: 500 })
+      }, initialCode)
-    await homePage.goToModelingScene()
+      await page.setBodyDimensions({ width: 1000, height: 500 })
-    await scene.settled(cmdBar)
+      await homePage.goToModelingScene()
-
+      await scene.settled(cmdBar)
-    await test.step('Double-click in feature tree and expect error toast', async () => {
+    })
+    await test.step('Edit fillet', async () => {
       await toolbar.openPane('feature-tree')
+      await toolbar.closePane('code')
       const operationButton = await toolbar.getFeatureTreeOperation('Fillet', 0)
       await operationButton.dblclick({ button: 'left' })
-      await expect(
+      await cmdBar.expectState({
-        page.getByText(
+        commandName: 'Fillet',
-          'Only chamfer and fillet in pipe expressions are supported for edits'
+        currentArgKey: 'radius',
-        )
+        currentArgValue: '5',
-      ).toBeVisible()
+        headerArguments: {
-      await page.waitForTimeout(1000)
+          Radius: '5',
+        },
+        highlightedHeaderArg: 'radius',
+        stage: 'arguments',
+      })
+      await page.keyboard.insertText('20')
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Radius: '20',
+        },
+        commandName: 'Fillet',
+      })
+      await cmdBar.progressCmdBar()
+    })
+    await test.step('Confirm changes', async () => {
+      await toolbar.openPane('code')
+      await toolbar.closePane('feature-tree')
+      await editor.expectEditor.toContain('radius = 20')
     })
   })
 

src/lang/modifyAst/addEdgeTreatment.test.ts

@@ -371,7 +371,7 @@ const runModifyAstCloneWithEdgeTreatmentAndTag = async (
   }
 
   // apply edge treatment to selection
-  const result = modifyAstWithEdgeTreatmentAndTag(
+  const result = await modifyAstWithEdgeTreatmentAndTag(
     ast,
     selection,
     parameters,

src/lang/modifyAst/addEdgeTreatment.ts

@@ -12,7 +12,6 @@ import {
   createLocalName,
   createPipeExpression,
 } from '@src/lang/create'
-import { updateModelingState } from '@src/lang/modelingWorkflows'
 import {
   getNodeFromPath,
   hasSketchPipeBeenExtruded,
@@ -39,7 +38,6 @@ import type {
   VariableDeclarator,
 } from '@src/lang/wasm'
 import type { KclCommandValue } from '@src/lib/commandTypes'
-import { EXECUTION_TYPE_REAL } from '@src/lib/constants'
 import type { Selection, Selections } from '@src/lib/selections'
 import { err } from '@src/lib/trap'
 import { isArray } from '@src/lib/utils'
@@ -65,43 +63,7 @@ export interface FilletParameters {
 export type EdgeTreatmentParameters = ChamferParameters | FilletParameters
 
 // Apply Edge Treatment (Fillet or Chamfer) To Selection
-export async function applyEdgeTreatmentToSelection(
+export async function modifyAstWithEdgeTreatmentAndTag(
-  ast: Node<Program>,
-  selection: Selections,
-  parameters: EdgeTreatmentParameters,
-  dependencies: {
-    kclManager: KclManager
-    engineCommandManager: EngineCommandManager
-    editorManager: EditorManager
-    codeManager: CodeManager
-  }
-): Promise<void | Error> {
-  // 1. clone and modify with edge treatment and tag
-  const result = modifyAstWithEdgeTreatmentAndTag(
-    ast,
-    selection,
-    parameters,
-    dependencies
-  )
-  if (err(result)) return result
-  const { modifiedAst, pathToEdgeTreatmentNode } = result
-
-  // 2. update ast
-  await updateModelingState(
-    modifiedAst,
-    EXECUTION_TYPE_REAL,
-    {
-      kclManager: dependencies.kclManager,
-      editorManager: dependencies.editorManager,
-      codeManager: dependencies.codeManager,
-    },
-    {
-      focusPath: pathToEdgeTreatmentNode,
-    }
-  )
-}
-
-export function modifyAstWithEdgeTreatmentAndTag(
   ast: Node<Program>,
   selections: Selections,
   parameters: EdgeTreatmentParameters,
@@ -111,9 +73,9 @@ export function modifyAstWithEdgeTreatmentAndTag(
     editorManager: EditorManager
     codeManager: CodeManager
   }
-):
+): Promise<
-  | { modifiedAst: Node<Program>; pathToEdgeTreatmentNode: Array<PathToNode> }
+  { modifiedAst: Node<Program>; pathToEdgeTreatmentNode: PathToNode[] } | Error
-  | Error {
+> {
   let clonedAst = structuredClone(ast)
   const clonedAstForGetExtrude = structuredClone(ast)
 
@@ -784,3 +746,47 @@ export async function deleteEdgeTreatment(
 
   return Error('Delete fillets not implemented')
 }
+
+// Edit Edge Treatment
+export async function editEdgeTreatment(
+  ast: Node<Program>,
+  selection: Selection,
+  parameters: EdgeTreatmentParameters
+): Promise<
+  { modifiedAst: Node<Program>; pathToEdgeTreatmentNode: PathToNode } | Error
+> {
+  // 1. clone and modify with new value
+  const modifiedAst = structuredClone(ast)
+
+  // find the edge treatment call
+  const edgeTreatmentCall = getNodeFromPath<CallExpressionKw>(
+    modifiedAst,
+    selection?.codeRef?.pathToNode,
+    'CallExpressionKw'
+  )
+  if (err(edgeTreatmentCall)) return edgeTreatmentCall
+
+  // edge treatment parameter
+  const parameterResult = getParameterNameAndValue(parameters)
+  if (err(parameterResult)) return parameterResult
+  const { parameterName, parameterValue } = parameterResult
+
+  // find the index of an argument to update
+  const index = edgeTreatmentCall.node.arguments.findIndex(
+    (arg) => arg.label.name === parameterName
+  )
+
+  // create a new argument with the updated value
+  const newArg = createLabeledArg(parameterName, parameterValue)
+
+  // if the parameter doesn't exist, add it; otherwise replace it
+  if (index === -1) {
+    edgeTreatmentCall.node.arguments.push(newArg)
+  } else {
+    edgeTreatmentCall.node.arguments[index] = newArg
+  }
+
+  let pathToEdgeTreatmentNode = selection?.codeRef?.pathToNode
+
+  return { modifiedAst, pathToEdgeTreatmentNode }
+}

src/lib/operations.ts

@@ -166,15 +166,6 @@ const prepareToEditEdgeTreatment: PrepareToEditCallback = async ({
     kclManager.ast,
     sourceRangeFromRust(operation.sourceRange)
   )
-  const isPipeExpression = nodeToEdit.some(
-    ([_, type]) => type === 'PipeExpression'
-  )
-  if (!isPipeExpression) {
-    return {
-      reason:
-        'Only chamfer and fillet in pipe expressions are supported for edits',
-    }
-  }
 
   let argDefaultValues:
     | ModelingCommandSchema['Chamfer']

src/machines/modelingMachine.ts

@@ -58,7 +58,8 @@ import type {
 } from '@src/lang/modifyAst/addEdgeTreatment'
 import {
   EdgeTreatmentType,
-  applyEdgeTreatmentToSelection,
+  modifyAstWithEdgeTreatmentAndTag,
+  editEdgeTreatment,
   getPathToExtrudeForSegmentSelection,
   mutateAstWithTagForSketchSegment,
 } from '@src/lang/modifyAst/addEdgeTreatment'
@@ -133,7 +134,6 @@ import {
 import type { ToolbarModeName } from '@src/lib/toolbar'
 import { err, reportRejection, trap } from '@src/lib/trap'
 import { uuidv4 } from '@src/lib/utils'
-import { deleteNodeInExtrudePipe } from '@src/lang/modifyAst/deleteNodeInExtrudePipe'
 import type { ImportStatement } from '@rust/kcl-lib/bindings/ImportStatement'
 
 export type SetSelections =
@@ -2311,18 +2311,107 @@ export const modelingMachine = setup({
 
         // Extract inputs
         const ast = kclManager.ast
+        let modifiedAst = structuredClone(ast)
+        let focusPath: PathToNode[] = []
         const { nodeToEdit, selection, radius } = input
 
-        // If this is an edit flow, first we're going to remove the old node
-        if (nodeToEdit) {
-          const oldNodeDeletion = deleteNodeInExtrudePipe(nodeToEdit, ast)
-          if (err(oldNodeDeletion)) return oldNodeDeletion
-        }
-
         const parameters: FilletParameters = {
           type: EdgeTreatmentType.Fillet,
           radius,
         }
+
+        const dependencies = {
+          kclManager,
+          engineCommandManager,
+          editorManager,
+          codeManager,
+        }
+
+        // Apply or edit fillet
+        if (nodeToEdit) {
+          // Edit existing fillet
+          // selection is not the edge treatment itself,
+          // but just the first edge in the fillet expression >
+          // we need to find the edgeCut artifact
+          // and build a new selection from it
+          // TODO: this is a bit of a hack, we should be able
+          // to get the edgeCut artifact from the selection
+          const firstSelection = selection.graphSelections[0]
+          const edgeCutArtifact = Array.from(
+            kclManager.artifactGraph.values()
+          ).find(
+            (artifact) =>
+              artifact.type === 'edgeCut' &&
+              artifact.consumedEdgeId === firstSelection.artifact?.id
+          )
+          if (!edgeCutArtifact || edgeCutArtifact.type !== 'edgeCut') {
+            return Promise.reject(
+              new Error(
+                'Failed to retrieve edgeCut artifact from sweepEdge selection'
+              )
+            )
+          }
+          const edgeTreatmentSelection = {
+            artifact: edgeCutArtifact,
+            codeRef: edgeCutArtifact.codeRef,
+          }
+
+          const editResult = await editEdgeTreatment(
+            ast,
+            edgeTreatmentSelection,
+            parameters
+          )
+          if (err(editResult)) return Promise.reject(editResult)
+
+          modifiedAst = editResult.modifiedAst
+          focusPath = [editResult.pathToEdgeTreatmentNode]
+        } else {
+          // Apply fillet to selection
+          const filletResult = await modifyAstWithEdgeTreatmentAndTag(
+            ast,
+            selection,
+            parameters,
+            dependencies
+          )
+          if (err(filletResult)) return Promise.reject(filletResult)
+          modifiedAst = filletResult.modifiedAst
+          focusPath = filletResult.pathToEdgeTreatmentNode
+        }
+
+        await updateModelingState(
+          modifiedAst,
+          EXECUTION_TYPE_REAL,
+          {
+            kclManager,
+            editorManager,
+            codeManager,
+          },
+          {
+            focusPath: focusPath,
+          }
+        )
+      }
+    ),
+    chamferAstMod: fromPromise(
+      async ({
+        input,
+      }: {
+        input: ModelingCommandSchema['Chamfer'] | undefined
+      }) => {
+        if (!input) {
+          return Promise.reject(new Error('No input provided'))
+        }
+
+        // Extract inputs
+        const ast = kclManager.ast
+        let modifiedAst = structuredClone(ast)
+        let focusPath: PathToNode[] = []
+        const { nodeToEdit, selection, length } = input
+
+        const parameters: ChamferParameters = {
+          type: EdgeTreatmentType.Chamfer,
+          length,
+        }
         const dependencies = {
           kclManager,
           engineCommandManager,
@@ -2330,14 +2419,69 @@ export const modelingMachine = setup({
           codeManager,
         }
 
-        // Apply fillet to selection
+        // Apply or edit chamfer
-        const filletResult = await applyEdgeTreatmentToSelection(
+        if (nodeToEdit) {
-          ast,
+          // Edit existing chamfer
-          selection,
+          // selection is not the edge treatment itself,
-          parameters,
+          // but just the first edge in the chamfer expression >
-          dependencies
+          // we need to find the edgeCut artifact
+          // and build a new selection from it
+          // TODO: this is a bit of a hack, we should be able
+          // to get the edgeCut artifact from the selection
+          const firstSelection = selection.graphSelections[0]
+          const edgeCutArtifact = Array.from(
+            kclManager.artifactGraph.values()
+          ).find(
+            (artifact) =>
+              artifact.type === 'edgeCut' &&
+              artifact.consumedEdgeId === firstSelection.artifact?.id
+          )
+          if (!edgeCutArtifact || edgeCutArtifact.type !== 'edgeCut') {
+            return Promise.reject(
+              new Error(
+                'Failed to retrieve edgeCut artifact from sweepEdge selection'
+              )
+            )
+          }
+          const edgeTreatmentSelection = {
+            artifact: edgeCutArtifact,
+            codeRef: edgeCutArtifact.codeRef,
+          }
+
+          const editResult = await editEdgeTreatment(
+            ast,
+            edgeTreatmentSelection,
+            parameters
+          )
+          if (err(editResult)) return Promise.reject(editResult)
+
+          modifiedAst = editResult.modifiedAst
+          focusPath = [editResult.pathToEdgeTreatmentNode]
+        } else {
+          // Apply chamfer to selection
+          const chamferResult = await modifyAstWithEdgeTreatmentAndTag(
+            ast,
+            selection,
+            parameters,
+            dependencies
+          )
+          if (err(chamferResult)) return Promise.reject(chamferResult)
+          modifiedAst = chamferResult.modifiedAst
+          focusPath = chamferResult.pathToEdgeTreatmentNode
+        }
+
+        await updateModelingState(
+          modifiedAst,
+          EXECUTION_TYPE_REAL,
+          {
+            kclManager,
+            editorManager,
+            codeManager,
+          },
+          {
+            focusPath: focusPath,
+          }
         )
-        if (err(filletResult)) return filletResult
       }
     ),
     'actor.parameter.create': fromPromise(
@@ -2461,47 +2605,6 @@ export const modelingMachine = setup({
         return {} as SketchDetailsUpdate
       }
     ),
-    chamferAstMod: fromPromise(
-      async ({
-        input,
-      }: {
-        input: ModelingCommandSchema['Chamfer'] | undefined
-      }) => {
-        if (!input) {
-          return new Error('No input provided')
-        }
-
-        // Extract inputs
-        const ast = kclManager.ast
-        const { nodeToEdit, selection, length } = input
-
-        // If this is an edit flow, first we're going to remove the old node
-        if (nodeToEdit) {
-          const oldNodeDeletion = deleteNodeInExtrudePipe(nodeToEdit, ast)
-          if (err(oldNodeDeletion)) return oldNodeDeletion
-        }
-
-        const parameters: ChamferParameters = {
-          type: EdgeTreatmentType.Chamfer,
-          length,
-        }
-        const dependencies = {
-          kclManager,
-          engineCommandManager,
-          editorManager,
-          codeManager,
-        }
-
-        // Apply chamfer to selection
-        const chamferResult = await applyEdgeTreatmentToSelection(
-          ast,
-          selection,
-          parameters,
-          dependencies
-        )
-        if (err(chamferResult)) return chamferResult
-      }
-    ),
     'submit-prompt-edit': fromPromise(
       async ({
         input,