Point-and-click Helix from cylinders (#5979)

e2e/playwright/point-click.spec.ts

@@ -1082,8 +1082,8 @@ openSketch = startSketchOn(XY)
   }) => {
     // One dumb hardcoded screen pixel value
     const testPoint = { x: 620, y: 257 }
-    const expectedOutput = `helix001 = helix(  revolutions = 1,  angleStart = 360,  ccw = false,  radius = 5,  axis = 'X',  length = 5,)`
-    const expectedLine = `revolutions=1,`
+    const expectedOutput = `helix001 = helix(  axis = 'X',  radius = 5,  length = 5,  revolutions = 1,  angleStart = 360,  ccw = false,)`
+    const expectedLine = `axis='X',`
 
     await homePage.goToModelingScene()
 
@@ -1091,17 +1091,17 @@ openSketch = startSketchOn(XY)
       await toolbar.helixButton.click()
       await cmdBar.expectState({
         stage: 'arguments',
-        currentArgKey: 'axisOrEdge',
+        currentArgKey: 'mode',
         currentArgValue: '',
         headerArguments: {
+          Mode: '',
           AngleStart: '',
-          AxisOrEdge: '',
-          CounterClockWise: '',
+          Revolutions: '',
           Length: '',
           Radius: '',
-          Revolutions: '',
+          CounterClockWise: '',
         },
-        highlightedHeaderArg: 'axisOrEdge',
+        highlightedHeaderArg: 'mode',
         commandName: 'Helix',
       })
       await cmdBar.progressCmdBar()
@@ -1110,7 +1110,19 @@ openSketch = startSketchOn(XY)
       await cmdBar.progressCmdBar()
       await cmdBar.progressCmdBar()
       await cmdBar.progressCmdBar()
-      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Mode: 'Axis',
+          Axis: 'X',
+          AngleStart: '360',
+          Revolutions: '1',
+          Length: '5',
+          Radius: '5',
+          CounterClockWise: '',
+        },
+        commandName: 'Helix',
+      })
       await cmdBar.progressCmdBar()
     })
 
@@ -1134,30 +1146,31 @@ openSketch = startSketchOn(XY)
       await cmdBar.expectState({
         commandName: 'Helix',
         stage: 'arguments',
-        currentArgKey: 'length',
-        currentArgValue: initialInput,
+        currentArgKey: 'CounterClockWise',
+        currentArgValue: '',
         headerArguments: {
-          AngleStart: '360',
           Axis: 'X',
-          CounterClockWise: '',
-          Length: initialInput,
-          Radius: '5',
+          AngleStart: '360',
           Revolutions: '1',
+          Radius: '5',
+          Length: initialInput,
+          CounterClockWise: '',
         },
-        highlightedHeaderArg: 'length',
+        highlightedHeaderArg: 'CounterClockWise',
       })
+      await page.keyboard.press('Shift+Backspace')
       await expect(cmdBar.currentArgumentInput).toBeVisible()
       await cmdBar.currentArgumentInput.locator('.cm-content').fill(newInput)
       await cmdBar.progressCmdBar()
       await cmdBar.expectState({
         stage: 'review',
         headerArguments: {
-          AngleStart: '360',
           Axis: 'X',
-          CounterClockWise: '',
-          Length: newInput,
-          Radius: '5',
+          AngleStart: '360',
           Revolutions: '1',
+          Radius: '5',
+          Length: newInput,
+          CounterClockWise: '',
         },
         commandName: 'Helix',
       })
@@ -1181,14 +1194,14 @@ openSketch = startSketchOn(XY)
     {
       selectionType: 'segment',
       testPoint: { x: 513, y: 221 },
-      expectedOutput: `helix001 = helix(  revolutions = 20,  angleStart = 0,  ccw = true,  radius = 1,  axis = seg01,  length = 100,)`,
-      expectedEditedOutput: `helix001 = helix(  revolutions = 20,  angleStart = 0,  ccw = true,  radius = 1,  axis = seg01,  length = 50,)`,
+      expectedOutput: `helix001 = helix(  axis = seg01,  radius = 1,  length = 100,  revolutions = 20,  angleStart = 0,  ccw = false,)`,
+      expectedEditedOutput: `helix001 = helix(  axis = seg01,  radius = 1,  length = 50,  revolutions = 20,  angleStart = 0,  ccw = false,)`,
     },
     {
       selectionType: 'sweepEdge',
       testPoint: { x: 564, y: 364 },
-      expectedOutput: `helix001 = helix(  revolutions = 20,  angleStart = 0,  ccw = true,  radius = 1,  axis =   getOppositeEdge(seg01),  length = 100,)`,
-      expectedEditedOutput: `helix001 = helix(  revolutions = 20,  angleStart = 0,  ccw = true,  radius = 1,  axis =   getOppositeEdge(seg01),  length = 50,)`,
+      expectedOutput: `helix001 = helix(  axis =   getOppositeEdge(seg01),  radius = 1,  length = 100,  revolutions = 20,  angleStart = 0,  ccw = false,)`,
+      expectedEditedOutput: `helix001 = helix(  axis =   getOppositeEdge(seg01),  radius = 1,  length = 50,  revolutions = 20,  angleStart = 0,  ccw = false,)`,
     },
   ]
   helixCases.map(
@@ -1225,17 +1238,17 @@ openSketch = startSketchOn(XY)
           await toolbar.helixButton.click()
           await cmdBar.expectState({
             stage: 'arguments',
-            currentArgKey: 'axisOrEdge',
+            currentArgKey: 'mode',
             currentArgValue: '',
             headerArguments: {
               AngleStart: '',
-              AxisOrEdge: '',
+              Mode: '',
               CounterClockWise: '',
               Length: '',
               Radius: '',
               Revolutions: '',
             },
-            highlightedHeaderArg: 'axisOrEdge',
+            highlightedHeaderArg: 'mode',
             commandName: 'Helix',
           })
           await cmdBar.selectOption({ name: 'Edge' }).click()
@@ -1246,7 +1259,6 @@ openSketch = startSketchOn(XY)
           await cmdBar.progressCmdBar()
           await page.keyboard.insertText('0')
           await cmdBar.progressCmdBar()
-          await cmdBar.selectOption({ name: 'True' }).click()
           await page.keyboard.insertText('1')
           await cmdBar.progressCmdBar()
           await page.keyboard.insertText('100')
@@ -1254,13 +1266,13 @@ openSketch = startSketchOn(XY)
           await cmdBar.expectState({
             stage: 'review',
             headerArguments: {
-              AngleStart: '0',
-              AxisOrEdge: 'Edge',
+              Mode: 'Edge',
               Edge: `1 ${selectionType}`,
-              CounterClockWise: '',
-              Length: '100',
-              Radius: '1',
+              AngleStart: '0',
               Revolutions: '20',
+              Radius: '1',
+              Length: '100',
+              CounterClockWise: '',
             },
             commandName: 'Helix',
           })
@@ -1285,17 +1297,18 @@ openSketch = startSketchOn(XY)
           await cmdBar.expectState({
             commandName: 'Helix',
             stage: 'arguments',
-            currentArgKey: 'length',
-            currentArgValue: initialInput,
+            currentArgKey: 'CounterClockWise',
+            currentArgValue: '',
             headerArguments: {
               AngleStart: '0',
-              CounterClockWise: '',
-              Length: initialInput,
-              Radius: '1',
               Revolutions: '20',
+              Radius: '1',
+              Length: initialInput,
+              CounterClockWise: '',
             },
-            highlightedHeaderArg: 'length',
+            highlightedHeaderArg: 'CounterClockWise',
           })
+          await page.keyboard.press('Shift+Backspace')
           await expect(cmdBar.currentArgumentInput).toBeVisible()
           await cmdBar.currentArgumentInput
             .locator('.cm-content')
@@ -1305,10 +1318,10 @@ openSketch = startSketchOn(XY)
             stage: 'review',
             headerArguments: {
               AngleStart: '0',
-              CounterClockWise: '',
-              Length: newInput,
-              Radius: '1',
               Revolutions: '20',
+              Radius: '1',
+              Length: newInput,
+              CounterClockWise: '',
             },
             commandName: 'Helix',
           })
@@ -1336,6 +1349,141 @@ openSketch = startSketchOn(XY)
     }
   )
 
+  test('Helix point-and-click on cylinder', async ({
+    context,
+    page,
+    homePage,
+    scene,
+    editor,
+    toolbar,
+    cmdBar,
+  }) => {
+    const initialCode = `sketch001 = startSketchOn(XY)
+profile001 = circle(
+  sketch001,
+  center = [0, 0],
+  radius = 100,
+  tag = $seg01,
+)
+extrude001 = extrude(profile001, length = 100)
+    `
+    await context.addInitScript((initialCode) => {
+      localStorage.setItem('persistCode', initialCode)
+    }, initialCode)
+    await page.setBodyDimensions({ width: 1000, height: 500 })
+    await homePage.goToModelingScene()
+    await scene.waitForExecutionDone()
+
+    // One dumb hardcoded screen pixel value
+    const testPoint = { x: 620, y: 257 }
+    const [clickOnWall] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
+    const expectedOutput = `helix001 = helix(  cylinder = extrude001,  revolutions = 1,  angleStart = 360,  ccw = false,)`
+    const expectedLine = `cylinder = extrude001,`
+    const expectedEditedOutput = `helix001 = helix(  cylinder = extrude001,  revolutions = 1,  angleStart = 360,  ccw = true,)`
+
+    await test.step(`Go through the command bar flow`, async () => {
+      await toolbar.helixButton.click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'mode',
+        currentArgValue: '',
+        headerArguments: {
+          Mode: '',
+          AngleStart: '',
+          Revolutions: '',
+          Length: '',
+          Radius: '',
+          CounterClockWise: '',
+        },
+        highlightedHeaderArg: 'mode',
+        commandName: 'Helix',
+      })
+      await cmdBar.selectOption({ name: 'Cylinder' }).click()
+      await cmdBar.expectState({
+        stage: 'arguments',
+        currentArgKey: 'cylinder',
+        currentArgValue: '',
+        headerArguments: {
+          Mode: 'Cylinder',
+          Cylinder: '',
+          AngleStart: '',
+          Revolutions: '',
+          CounterClockWise: '',
+        },
+        highlightedHeaderArg: 'cylinder',
+        commandName: 'Helix',
+      })
+      await clickOnWall()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.progressCmdBar()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          Mode: 'Cylinder',
+          Cylinder: '1 face',
+          AngleStart: '360',
+          Revolutions: '1',
+          CounterClockWise: '',
+        },
+        commandName: 'Helix',
+      })
+      await cmdBar.progressCmdBar()
+    })
+
+    await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
+      await editor.expectEditor.toContain(expectedOutput)
+      await editor.expectState({
+        diagnostics: [],
+        activeLines: [expectedLine],
+        highlightedCode: '',
+      })
+    })
+
+    await test.step(`Edit helix through the feature tree`, async () => {
+      await editor.closePane()
+      const operationButton = await toolbar.getFeatureTreeOperation('Helix', 0)
+      await operationButton.dblclick()
+      await cmdBar.expectState({
+        commandName: 'Helix',
+        stage: 'arguments',
+        currentArgKey: 'CounterClockWise',
+        currentArgValue: '',
+        headerArguments: {
+          AngleStart: '360',
+          Revolutions: '1',
+          CounterClockWise: '',
+        },
+        highlightedHeaderArg: 'CounterClockWise',
+      })
+      await cmdBar.selectOption({ name: 'True' }).click()
+      await cmdBar.expectState({
+        stage: 'review',
+        headerArguments: {
+          AngleStart: '360',
+          Revolutions: '1',
+          CounterClockWise: 'true',
+        },
+        commandName: 'Helix',
+      })
+      await cmdBar.progressCmdBar()
+      await toolbar.closePane('feature-tree')
+      await toolbar.openPane('code')
+      await editor.expectEditor.toContain(expectedEditedOutput)
+      await editor.closePane()
+    })
+
+    await test.step('Delete helix via feature tree selection', async () => {
+      await toolbar.openPane('feature-tree')
+      const operationButton = await toolbar.getFeatureTreeOperation('Helix', 0)
+      await operationButton.click({ button: 'left' })
+      await page.keyboard.press('Delete')
+      await toolbar.closePane('feature-tree')
+      await toolbar.openPane('code')
+      await editor.expectEditor.not.toContain(expectedEditedOutput)
+    })
+  })
+
   const loftPointAndClickCases = [
     { shouldPreselect: true },
     { shouldPreselect: false },

src/components/CommandBar/CommandBarKclInput.tsx

@@ -80,7 +80,8 @@ function CommandBarKclInput({
         : arg.variableName
     }
     // or derive it from the previously set value or the argument name
-    return previouslySetValue && 'variableName' in previouslySetValue
+    return typeof previouslySetValue === 'object' &&
+      'variableName' in previouslySetValue
       ? previouslySetValue.variableName
       : arg.name
   }, [
@@ -96,7 +97,8 @@ function CommandBarKclInput({
   )
   const [value, setValue] = useState(initialValue)
   const [createNewVariable, setCreateNewVariable] = useState(
-    (previouslySetValue && 'variableName' in previouslySetValue) ||
+    (typeof previouslySetValue === 'object' &&
+      'variableName' in previouslySetValue) ||
       arg.createVariable === 'byDefault' ||
       arg.createVariable === 'force' ||
       false
@@ -132,7 +134,8 @@ function CommandBarKclInput({
     selection: {
       anchor: 0,
       head:
-        previouslySetValue && 'valueText' in previouslySetValue
+        typeof previouslySetValue === 'object' &&
+        'valueText' in previouslySetValue
           ? previouslySetValue.valueText.length
           : defaultValue.length,
     },

src/lang/modifyAst.ts

@@ -811,40 +811,53 @@ export function addOffsetPlane({
  */
 export function addHelix({
   node,
+  axis,
+  cylinder,
   revolutions,
   angleStart,
-  ccw,
   radius,
-  axis,
   length,
+  ccw,
   insertIndex,
   variableName,
 }: {
   node: Node<Program>
+  axis?: Node<Literal> | Node<Name | CallExpression | CallExpressionKw>
+  cylinder?: VariableDeclarator
   revolutions: Expr
   angleStart: Expr
+  radius?: Expr
+  length?: Expr
   ccw: boolean
-  radius: Expr
-  axis: Node<Literal> | Node<Name | CallExpression | CallExpressionKw>
-  length: Expr
   insertIndex?: number
   variableName?: string
 }): { modifiedAst: Node<Program>; pathToNode: PathToNode } {
   const modifiedAst = structuredClone(node)
   const name =
     variableName ?? findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.HELIX)
+  const modeArgs: CallExpressionKw['arguments'] = []
+  if (axis && radius) {
+    modeArgs.push(createLabeledArg('axis', axis))
+    modeArgs.push(createLabeledArg('radius', radius))
+    if (length) {
+      modeArgs.push(createLabeledArg('length', length))
+    }
+  } else if (cylinder) {
+    modeArgs.push(
+      createLabeledArg('cylinder', createLocalName(cylinder.id.name))
+    )
+  }
+
   const variable = createVariableDeclaration(
     name,
     createCallExpressionStdLibKw(
       'helix',
       null, // Not in a pipeline
       [
+        ...modeArgs,
         createLabeledArg('revolutions', revolutions),
         createLabeledArg('angleStart', angleStart),
         createLabeledArg('ccw', createLiteral(ccw)),
-        createLabeledArg('radius', radius),
-        createLabeledArg('axis', axis),
-        createLabeledArg('length', length),
       ]
     )
   )

src/lib/commandBarConfigs/modelingCommandConfig.ts

@@ -38,6 +38,8 @@ export const EXTRUSION_RESULTS = [
 
 export const COMMAND_APPEARANCE_COLOR_DEFAULT = 'default'
 
+export type HelixModes = 'Axis' | 'Edge' | 'Cylinder'
+
 export type ModelingCommandSchema = {
   'Enter sketch': {}
   Export: {
@@ -103,15 +105,17 @@ export type ModelingCommandSchema = {
     // Enables editing workflow
     nodeToEdit?: PathToNode
     // Flow arg
-    axisOrEdge: 'Axis' | 'Edge'
+    mode: HelixModes
+    // Three different arguments depending on mode
+    axis?: string
+    edge?: Selections
+    cylinder?: Selections
     // KCL stdlib arguments
-    axis: string | undefined
-    edge: Selections | undefined
     revolutions: KclCommandValue
     angleStart: KclCommandValue
-    ccw: boolean
-    radius: KclCommandValue
-    length: KclCommandValue
+    radius?: KclCommandValue // axis or edge modes only
+    length?: KclCommandValue // axis or edge modes only
+    ccw: boolean // optional boolean argument, default value to false
   }
   'event.parameter.create': {
     value: KclCommandValue
@@ -530,7 +534,6 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
   Helix: {
     description: 'Create a helix or spiral in 3D about an axis.',
     icon: 'helix',
-    status: 'development',
     needsReview: true,
     args: {
       nodeToEdit: {
@@ -541,69 +544,91 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
         required: false,
         hidden: true,
       },
-      axisOrEdge: {
+      mode: {
         inputType: 'options',
         required: true,
         defaultValue: 'Axis',
         options: [
           { name: 'Axis', isCurrent: true, value: 'Axis' },
           { name: 'Edge', isCurrent: false, value: 'Edge' },
+          { name: 'Cylinder', isCurrent: false, value: 'Cylinder' },
         ],
         hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
       },
       axis: {
         inputType: 'options',
         required: (commandContext) =>
-          ['Axis'].includes(
-            commandContext.argumentsToSubmit.axisOrEdge as string
-          ),
+          ['Axis'].includes(commandContext.argumentsToSubmit.mode as string),
         options: [
           { name: 'X Axis', value: 'X' },
           { name: 'Y Axis', value: 'Y' },
           { name: 'Z Axis', value: 'Z' },
         ],
+        hidden: false, // for consistency here, we can actually edit here since it's not a selection
       },
       edge: {
         required: (commandContext) =>
-          ['Edge'].includes(
-            commandContext.argumentsToSubmit.axisOrEdge as string
-          ),
+          ['Edge'].includes(commandContext.argumentsToSubmit.mode as string),
         inputType: 'selection',
         selectionTypes: ['segment', 'sweepEdge'],
         multiple: false,
         hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
       },
+      cylinder: {
+        required: (commandContext) =>
+          ['Cylinder'].includes(
+            commandContext.argumentsToSubmit.mode as string
+          ),
+        inputType: 'selection',
+        selectionTypes: ['wall'],
+        multiple: false,
+        hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
+      },
       revolutions: {
         inputType: 'kcl',
         defaultValue: '1',
         required: true,
-        warningMessage:
-          'The helix workflow is new and under tested. Please break it and report issues.',
       },
       angleStart: {
         inputType: 'kcl',
         defaultValue: KCL_DEFAULT_DEGREE,
         required: true,
       },
-      ccw: {
-        inputType: 'options',
-        required: true,
-        displayName: 'CounterClockWise',
-        defaultValue: false,
-        options: [
-          { name: 'False', value: false },
-          { name: 'True', value: true },
-        ],
-      },
       radius: {
         inputType: 'kcl',
         defaultValue: KCL_DEFAULT_LENGTH,
-        required: true,
+        required: (commandContext) =>
+          !['Cylinder'].includes(
+            commandContext.argumentsToSubmit.mode as string
+          ),
       },
       length: {
         inputType: 'kcl',
         defaultValue: KCL_DEFAULT_LENGTH,
+        required: (commandContext) =>
+          !['Cylinder'].includes(
+            commandContext.argumentsToSubmit.mode as string
+          ),
+      },
+      ccw: {
+        inputType: 'options',
+        skip: true,
         required: true,
+        defaultValue: false,
+        valueSummary: (value) => String(value),
+        displayName: 'CounterClockWise',
+        options: (commandContext) => [
+          {
+            name: 'False',
+            value: false,
+            isCurrent: !Boolean(commandContext.argumentsToSubmit.ccw),
+          },
+          {
+            name: 'True',
+            value: true,
+            isCurrent: Boolean(commandContext.argumentsToSubmit.ccw),
+          },
+        ],
       },
     },
   },

src/lib/operations.ts

@@ -5,6 +5,7 @@ import {
   getCapCodeRef,
   getEdgeCutConsumedCodeRef,
   getSweepEdgeCodeRef,
+  getWallCodeRef,
 } from 'lang/std/artifactGraph'
 import { Operation } from '@rust/kcl-lib/bindings/Operation'
 import { codeManager, engineCommandManager, kclManager } from './singletons'
@@ -13,10 +14,14 @@ import { getNodePathFromSourceRange } from 'lang/queryAstNodePathUtils'
 import { sourceRangeFromRust } from 'lang/wasm'
 import { CommandBarMachineEvent } from 'machines/commandBarMachine'
 import { stringToKclExpression } from './kclHelpers'
-import { ModelingCommandSchema } from './commandBarConfigs/modelingCommandConfig'
+import {
+  HelixModes,
+  ModelingCommandSchema,
+} from './commandBarConfigs/modelingCommandConfig'
 import { isDefaultPlaneStr } from './planes'
 import { Selection, Selections } from './selections'
 import { rustContext } from './singletons'
+import { KclExpression } from './commandTypes'
 
 type ExecuteCommandEvent = CommandBarMachineEvent & {
   type: 'Find and select command'
@@ -571,27 +576,32 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
     groupId: 'modeling',
   }
   if (operation.type !== 'StdLibCall' || !operation.labeledArgs) {
-    return baseCommand
+    return { reason: 'Wrong operation type or arguments' }
   }
 
-  // TODO: find a way to loop over the arguments while keeping it safe
-
-  // axis options string arg
-  if (!('axis' in operation.labeledArgs) || !operation.labeledArgs.axis) {
-    return baseCommand
-  }
-
-  const axisValue = operation.labeledArgs.axis.value
-  let axisOrEdge: 'Axis' | 'Edge' | undefined
+  // Flow arg
+  let mode: HelixModes | undefined
+  // Three different arguments depending on mode
   let axis: string | undefined
   let edge: Selections | undefined
+  let cylinder: Selections | undefined
+  // Rest of stdlib args
+  let revolutions: KclExpression | undefined // common to all modes, can't remain undefined
+  let angleStart: KclExpression | undefined // common to all modes, can't remain undefined
+  let length: KclExpression | undefined // axis or edge modes only
+  let radius: KclExpression | undefined // axis or edge modes only
+  let ccw = false // optional boolean argument, default value
+
+  if ('axis' in operation.labeledArgs && operation.labeledArgs.axis) {
+    // axis options string or selection arg
+    const axisValue = operation.labeledArgs.axis.value
     if (axisValue.type === 'String') {
       // default axis casee
-    axisOrEdge = 'Axis'
+      mode = 'Axis'
       axis = axisValue.value
     } else if (axisValue.type === 'TagIdentifier' && axisValue.artifact_id) {
       // segment case
-    axisOrEdge = 'Edge'
+      mode = 'Edge'
       const artifact = getArtifactOfTypes(
         {
           key: axisValue.artifact_id,
@@ -600,7 +610,7 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
         engineCommandManager.artifactGraph
       )
       if (err(artifact)) {
-      return baseCommand
+        return { reason: "Couldn't find related edge artifact" }
       }
 
       edge = {
@@ -614,7 +624,7 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
       }
     } else if (axisValue.type === 'Uuid') {
       // sweepEdge case
-    axisOrEdge = 'Edge'
+      mode = 'Edge'
       const artifact = getArtifactOfTypes(
         {
           key: axisValue.value,
@@ -623,7 +633,7 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
         engineCommandManager.artifactGraph
       )
       if (err(artifact)) {
-      return baseCommand
+        return { reason: "Couldn't find related edge artifact" }
       }
 
       const codeRef = getSweepEdgeCodeRef(
@@ -631,7 +641,7 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
         engineCommandManager.artifactGraph
       )
       if (err(codeRef)) {
-      return baseCommand
+        return { reason: "Couldn't find related edge code ref" }
       }
 
       edge = {
@@ -644,94 +654,148 @@ const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
         otherSelections: [],
       }
     } else {
-    return baseCommand
+      return { reason: 'The type of the axis argument is unsupported' }
+    }
+  } else if (
+    'cylinder' in operation.labeledArgs &&
+    operation.labeledArgs.cylinder
+  ) {
+    mode = 'Cylinder'
+    // axis cylinder selection arg
+    if (operation.labeledArgs.cylinder.value.type !== 'Solid') {
+      return { reason: "Cylinder arg found isn't of type Solid" }
     }
 
-  // revolutions kcl arg
-  if (
-    !('revolutions' in operation.labeledArgs) ||
-    !operation.labeledArgs.revolutions
-  ) {
-    return baseCommand
+    const sweepId = operation.labeledArgs.cylinder.value.value.artifactId
+    const wallArtifact = [...engineCommandManager.artifactGraph.values()].find(
+      (p) => p.type === 'wall' && p.sweepId === sweepId
+    )
+    if (!wallArtifact || wallArtifact.type !== 'wall') {
+      return {
+        reason: "Cylinder arg found doesn't point to a valid sweep wall",
       }
-  const revolutions = await stringToKclExpression(
+    }
+
+    const wallCodeRef = getWallCodeRef(
+      wallArtifact,
+      engineCommandManager.artifactGraph
+    )
+    if (err(wallCodeRef)) {
+      return {
+        reason: "Cylinder arg found doesn't point to a valid sweep code ref",
+      }
+    }
+
+    cylinder = {
+      graphSelections: [
+        {
+          artifact: wallArtifact,
+          codeRef: wallCodeRef,
+        },
+      ],
+      otherSelections: [],
+    }
+  } else {
+    return {
+      reason: "The axis or cylinder arguments couldn't be prepared for edit",
+    }
+  }
+
+  // revolutions kcl arg (common for all)
+  if (
+    'revolutions' in operation.labeledArgs &&
+    operation.labeledArgs.revolutions
+  ) {
+    const r = await stringToKclExpression(
       codeManager.code.slice(
         operation.labeledArgs.revolutions.sourceRange[0],
         operation.labeledArgs.revolutions.sourceRange[1]
       )
     )
-  if (err(revolutions) || 'errors' in revolutions) return baseCommand
-
-  // angleStart kcl arg
-  if (
-    !('angleStart' in operation.labeledArgs) ||
-    !operation.labeledArgs.angleStart
-  ) {
-    return baseCommand
+    if (err(r) || 'errors' in r) {
+      return { reason: 'Errors found in revolutions argument' }
     }
-  const angleStart = await stringToKclExpression(
+
+    revolutions = r
+  } else {
+    return { reason: "Couldn't find revolutions argument" }
+  }
+
+  // angleStart kcl arg (common for all)
+  if (
+    'angleStart' in operation.labeledArgs &&
+    operation.labeledArgs.angleStart
+  ) {
+    const r = await stringToKclExpression(
       codeManager.code.slice(
         operation.labeledArgs.angleStart.sourceRange[0],
         operation.labeledArgs.angleStart.sourceRange[1]
       )
     )
-  if (err(angleStart) || 'errors' in angleStart) {
-    return baseCommand
+    if (err(r) || 'errors' in r) {
+      return { reason: 'Errors found in angleStart argument' }
     }
 
-  // counterClockWise options boolean arg
-  if (!('ccw' in operation.labeledArgs) || !operation.labeledArgs.ccw) {
-    return baseCommand
+    angleStart = r
+  } else {
+    return { reason: "Couldn't find angleStart argument" }
   }
-  const ccw =
-    codeManager.code.slice(
-      operation.labeledArgs.ccw.sourceRange[0],
-      operation.labeledArgs.ccw.sourceRange[1]
-    ) === 'true'
 
-  // radius kcl arg
-  if (!('radius' in operation.labeledArgs) || !operation.labeledArgs.radius) {
-    console.log(
-      "!('radius' in operation.labeledArgs) || !operation.labeledArgs.radius"
-    )
-    return baseCommand
-  }
-  const radius = await stringToKclExpression(
+  // radius and cylinder and kcl arg (only for axis or edge)
+  if (mode !== 'Cylinder') {
+    if ('radius' in operation.labeledArgs && operation.labeledArgs.radius) {
+      const r = await stringToKclExpression(
         codeManager.code.slice(
           operation.labeledArgs.radius.sourceRange[0],
           operation.labeledArgs.radius.sourceRange[1]
         )
       )
-  if (err(radius) || 'errors' in radius) {
-    return baseCommand
+      if (err(r) || 'errors' in r) {
+        return { reason: 'Error in radius argument retrieval' }
+      }
+      radius = r
+    } else {
+      return { reason: "Couldn't find radius argument" }
     }
 
-  // length kcl arg
-  if (!('length' in operation.labeledArgs) || !operation.labeledArgs.length) {
-    return baseCommand
-  }
-  const length = await stringToKclExpression(
+    if ('length' in operation.labeledArgs && operation.labeledArgs.length) {
+      const r = await stringToKclExpression(
         codeManager.code.slice(
           operation.labeledArgs.length.sourceRange[0],
           operation.labeledArgs.length.sourceRange[1]
         )
       )
-  if (err(length) || 'errors' in length) {
-    return baseCommand
+      if (err(r) || 'errors' in r) {
+        return { reason: 'Error in length argument retrieval' }
+      }
+      length = r
+    } else {
+      return { reason: "Couldn't find length argument" }
+    }
+  }
+
+  // counterClockWise boolean arg (optional)
+  if ('ccw' in operation.labeledArgs && operation.labeledArgs.ccw) {
+    ccw =
+      codeManager.code.slice(
+        operation.labeledArgs.ccw.sourceRange[0],
+        operation.labeledArgs.ccw.sourceRange[1]
+      ) === 'true'
   }
 
   // Assemble the default argument values for the Offset Plane command,
   // with `nodeToEdit` set, which will let the Offset Plane actor know
   // to edit the node that corresponds to the StdLibCall.
   const argDefaultValues: ModelingCommandSchema['Helix'] = {
-    axisOrEdge,
+    mode,
     axis,
     edge,
+    cylinder,
     revolutions,
     angleStart,
-    ccw,
     radius,
     length,
+    ccw,
     nodeToEdit: getNodePathFromSourceRange(
       kclManager.ast,
       sourceRangeFromRust(operation.sourceRange)

src/machines/commandBarMachine.ts

@@ -366,6 +366,7 @@ export const commandBarMachine = setup({
                 !(argConfig.inputType === 'kcl' || argConfig.skip)
               const hasInvalidKclValue =
                 argConfig.inputType === 'kcl' &&
+                isRequired &&
                 !(argValue as Partial<KclCommandValue> | undefined)?.valueAst
               const hasInvalidOptionsValue =
                 isRequired &&

src/machines/modelingMachine.ts

@@ -1,5 +1,9 @@
 import {
+  CallExpression,
+  CallExpressionKw,
   Expr,
+  Literal,
+  Name,
   PathToNode,
   VariableDeclaration,
   VariableDeclarator,
@@ -1910,11 +1914,13 @@ export const modelingMachine = setup({
       }) => {
         if (!input) return new Error('No input provided')
         // Extract inputs
+        console.log('input', input)
         const ast = kclManager.ast
         const {
-          axisOrEdge,
+          mode,
           axis,
           edge,
+          cylinder,
           revolutions,
           angleStart,
           ccw,
@@ -1946,16 +1952,50 @@ export const modelingMachine = setup({
           opInsertIndex = nodeToEdit[1][0]
         }
 
-        const getAxisResult = getAxisExpressionAndIndex(
-          axisOrEdge,
-          axis,
-          edge,
-          ast
+        let cylinderDeclarator: VariableDeclarator | undefined
+        let axisExpression:
+          | Node<CallExpression | CallExpressionKw | Name>
+          | Node<Literal>
+          | undefined
+
+        if (mode === 'Cylinder') {
+          if (
+            !(
+              cylinder &&
+              cylinder.graphSelections[0] &&
+              cylinder.graphSelections[0].artifact?.type === 'wall'
+            )
+          ) {
+            return new Error('Cylinder argument not valid')
+          }
+          const clonedAstForGetExtrude = structuredClone(ast)
+          const extrudeLookupResult = getPathToExtrudeForSegmentSelection(
+            clonedAstForGetExtrude,
+            cylinder.graphSelections[0],
+            engineCommandManager.artifactGraph
+          )
+          if (err(extrudeLookupResult)) {
+            return extrudeLookupResult
+          }
+          const extrudeNode = getNodeFromPath<VariableDeclaration>(
+            ast,
+            extrudeLookupResult.pathToExtrudeNode,
+            'VariableDeclaration'
+          )
+          if (err(extrudeNode)) {
+            return extrudeNode
+          }
+          cylinderDeclarator = extrudeNode.node.declaration
+        } else if (mode === 'Axis' || mode === 'Edge') {
+          const getAxisResult = getAxisExpressionAndIndex(mode, axis, edge, ast)
+          if (err(getAxisResult)) {
+            return getAxisResult
+          }
+          axisExpression = getAxisResult.generatedAxis
+        } else {
+          return new Error(
+            'Generated axis or cylinder declarator selection is missing.'
           )
-        if (err(getAxisResult)) return getAxisResult
-        const { generatedAxis } = getAxisResult
-        if (!generatedAxis) {
-          return new Error('Generated axis selection is missing.')
         }
 
         // TODO: figure out if we want to smart insert after the sketch as below
@@ -1965,13 +2005,13 @@ export const modelingMachine = setup({
         // opInsertIndex = axisIndexIfAxis + 1
         // }
 
-        for (const variable of [revolutions, angleStart, radius, length]) {
+        for (const v of [revolutions, angleStart, radius, length]) {
+          if (v === undefined) {
+            continue
+          }
+          const variable = v as KclCommandValue
           // Insert the variable if it exists
-          if (
-            'variableName' in variable &&
-            variable.variableName &&
-            variable.insertIndex !== undefined
-          ) {
+          if ('variableName' in variable && variable.variableName) {
             const newBody = [...ast.body]
             newBody.splice(
               variable.insertIndex,
@@ -1982,19 +2022,21 @@ export const modelingMachine = setup({
           }
         }
 
-        const valueOrVariable = (variable: KclCommandValue) =>
-          'variableName' in variable
+        const valueOrVariable = (variable: KclCommandValue) => {
+          return 'variableName' in variable
             ? variable.variableIdentifierAst
             : variable.valueAst
+        }
 
         const { modifiedAst, pathToNode } = addHelix({
           node: ast,
           revolutions: valueOrVariable(revolutions),
           angleStart: valueOrVariable(angleStart),
           ccw,
-          radius: valueOrVariable(radius),
-          axis: generatedAxis,
-          length: valueOrVariable(length),
+          radius: radius ? valueOrVariable(radius) : undefined,
+          axis: axisExpression,
+          cylinder: cylinderDeclarator,
+          length: length ? valueOrVariable(length) : undefined,
           insertIndex: opInsertIndex,
           variableName: opVariableName,
         })