Add rectangle based sweep test case

Fixes #5301

e2e/playwright/point-click.spec.ts

@@ -1333,98 +1333,166 @@ loft001 = loft([sketch001, sketch002])
     })
   })
 
-  test(`Sweep point-and-click`, async ({
+  const sweepCases = [
-    context,
+    {
-    page,
+      targetType: 'circle',
-    homePage,
+      testPoint: { x: 700, y: 250 },
-    scene,
+      initialCode: `sketch001 = startSketchOn('YZ')
-    editor,
+profile001 = circle(sketch001, center = [0, 0], radius = 500)
-    toolbar,
-    cmdBar,
-  }) => {
-    const initialCode = `sketch001 = startSketchOn('YZ')
-  |> circle(
-       center = [0, 0],
-       radius = 500
-     )
 sketch002 = startSketchOn('XZ')
   |> startProfileAt([0, 0], %)
   |> xLine(length = -500)
-  |> tangentialArcTo([-2000, 500], %)
+  |> tangentialArcTo([-2000, 500], %)`,
-`
+    },
-    await context.addInitScript((initialCode) => {
+    {
-      localStorage.setItem('persistCode', initialCode)
+      targetType: 'rectangle',
-    }, initialCode)
+      testPoint: { x: 710, y: 255 },
-    await page.setBodyDimensions({ width: 1000, height: 500 })
+      initialCode: `sketch001 = startSketchOn('YZ')
-    await homePage.goToModelingScene()
+profile001 = startProfileAt([-400, -400], sketch001)
-    await scene.waitForExecutionDone()
+  |> angledLine([0, 800], %, $rectangleSegmentA001)
+  |> angledLine([
+       segAng(rectangleSegmentA001) + 90,
+       800
+     ], %)
+  |> angledLine([
+       segAng(rectangleSegmentA001),
+       -segLen(rectangleSegmentA001)
+     ], %)
+  |> line(endAbsolute = [profileStartX(%), profileStartY(%)])
+  |> close()
+sketch002 = startSketchOn('XZ')
+  |> startProfileAt([0, 0], %)
+  |> xLine(length = -500)
+  |> tangentialArcTo([-2000, 500], %)`,
+    },
+  ]
+  sweepCases.map(({ initialCode, targetType, testPoint }) => {
+    test(`Sweep point-and-click ${targetType}`, async ({
+      context,
+      page,
+      homePage,
+      scene,
+      editor,
+      toolbar,
+      cmdBar,
+    }) => {
+      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
+      // One dumb hardcoded screen pixel value
-    const testPoint = { x: 700, y: 250 }
+      const [clickOnSketch1] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
-    const [clickOnSketch1] = scene.makeMouseHelpers(testPoint.x, testPoint.y)
+      const [clickOnSketch2] = scene.makeMouseHelpers(
-    const [clickOnSketch2] = scene.makeMouseHelpers(
+        testPoint.x - 50,
-      testPoint.x - 50,
+        testPoint.y
-      testPoint.y
+      )
-    )
+      const sweepDeclaration =
-    const sweepDeclaration = 'sweep001 = sweep(sketch001, path = sketch002)'
+        'sweep001 = sweep(profile001, path = sketch002, sectional = false)'
+      const editedSweepDeclaration =
+        'sweep001 = sweep(profile001, path = sketch002, sectional = true)'
 
-    await test.step(`Look for sketch001`, async () => {
+      await test.step(`Look for sketch001`, async () => {
-      await toolbar.closePane('code')
+        await toolbar.closePane('code')
-      await scene.expectPixelColor([53, 53, 53], testPoint, 15)
+        await scene.expectPixelColor([53, 53, 53], testPoint, 15)
-    })
-
-    await test.step(`Go through the command bar flow`, async () => {
-      await toolbar.sweepButton.click()
-      await cmdBar.expectState({
-        commandName: 'Sweep',
-        currentArgKey: 'target',
-        currentArgValue: '',
-        headerArguments: {
-          Target: '',
-          Trajectory: '',
-        },
-        highlightedHeaderArg: 'target',
-        stage: 'arguments',
       })
-      await clickOnSketch1()
-      await cmdBar.expectState({
-        commandName: 'Sweep',
-        currentArgKey: 'trajectory',
-        currentArgValue: '',
-        headerArguments: {
-          Target: '1 face',
-          Trajectory: '',
-        },
-        highlightedHeaderArg: 'trajectory',
-        stage: 'arguments',
-      })
-      await clickOnSketch2()
-      await page.waitForTimeout(500)
-      await cmdBar.progressCmdBar()
-      await toolbar.openPane('code')
-      await page.waitForTimeout(500)
-    })
 
-    await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
+      await test.step(`Go through the command bar flow`, async () => {
-      // await scene.expectPixelColor([135, 64, 73], testPoint, 15) // FIXME
+        await toolbar.sweepButton.click()
-      await editor.expectEditor.toContain(sweepDeclaration)
+        await cmdBar.expectState({
-      await editor.expectState({
+          commandName: 'Sweep',
-        diagnostics: [],
+          currentArgKey: 'target',
-        activeLines: [sweepDeclaration],
+          currentArgValue: '',
-        highlightedCode: '',
+          headerArguments: {
+            Sectional: '',
+            Target: '',
+            Trajectory: '',
+          },
+          highlightedHeaderArg: 'target',
+          stage: 'arguments',
+        })
+        await clickOnSketch1()
+        await cmdBar.expectState({
+          commandName: 'Sweep',
+          currentArgKey: 'trajectory',
+          currentArgValue: '',
+          headerArguments: {
+            Sectional: '',
+            Target: '1 face',
+            Trajectory: '',
+          },
+          highlightedHeaderArg: 'trajectory',
+          stage: 'arguments',
+        })
+        await clickOnSketch2()
+        await page.waitForTimeout(500)
+        await cmdBar.progressCmdBar()
+        await cmdBar.expectState({
+          commandName: 'Sweep',
+          headerArguments: {
+            Target: '1 face',
+            Trajectory: '1 segment',
+            Sectional: '',
+          },
+          stage: 'review',
+        })
+        await cmdBar.progressCmdBar()
       })
-      await toolbar.closePane('code')
-    })
 
-    await test.step('Delete sweep via feature tree selection', async () => {
+      await test.step(`Confirm code is added to the editor, scene has changed`, async () => {
-      await toolbar.openPane('feature-tree')
+        await toolbar.openPane('code')
-      await page.waitForTimeout(500)
+        await editor.expectEditor.toContain(sweepDeclaration)
-      const operationButton = await toolbar.getFeatureTreeOperation('Sweep', 0)
+        await scene.expectPixelColor([120, 120, 120], testPoint, 40)
-      await operationButton.click({ button: 'left' })
+        await toolbar.closePane('code')
-      await page.keyboard.press('Delete')
+      })
-      await page.waitForTimeout(500)
+
-      await toolbar.closePane('feature-tree')
+      await test.step('Edit sweep via feature tree selection works', async () => {
-      await scene.expectPixelColor([53, 53, 53], testPoint, 15)
+        await toolbar.openPane('feature-tree')
+        const operationButton = await toolbar.getFeatureTreeOperation(
+          'Sweep',
+          0
+        )
+        await operationButton.dblclick({ button: 'left' })
+        await cmdBar.expectState({
+          commandName: 'Sweep',
+          currentArgKey: 'sectional',
+          currentArgValue: '',
+          headerArguments: {
+            Sectional: '',
+          },
+          highlightedHeaderArg: 'sectional',
+          stage: 'arguments',
+        })
+        await cmdBar.selectOption({ name: 'True' }).click()
+        await cmdBar.expectState({
+          commandName: 'Sweep',
+          headerArguments: {
+            Sectional: '',
+          },
+          stage: 'review',
+        })
+        await cmdBar.progressCmdBar()
+        await toolbar.closePane('feature-tree')
+        await toolbar.openPane('code')
+        await editor.expectEditor.toContain(editedSweepDeclaration)
+        await toolbar.closePane('code')
+      })
+
+      await test.step('Delete sweep via feature tree selection', async () => {
+        await toolbar.openPane('feature-tree')
+        await page.waitForTimeout(500)
+        const operationButton = await toolbar.getFeatureTreeOperation(
+          'Sweep',
+          0
+        )
+        await operationButton.click({ button: 'left' })
+        await page.keyboard.press('Delete')
+        await page.waitForTimeout(500)
+        await toolbar.closePane('feature-tree')
+        await scene.expectPixelColor([53, 53, 53], testPoint, 15)
+      })
     })
   })
 
@@ -1473,6 +1541,7 @@ sketch002 = startSketchOn('XZ')
         currentArgKey: 'target',
         currentArgValue: '',
         headerArguments: {
+          Sectional: '',
           Target: '',
           Trajectory: '',
         },
@@ -1485,6 +1554,7 @@ sketch002 = startSketchOn('XZ')
         currentArgKey: 'trajectory',
         currentArgValue: '',
         headerArguments: {
+          Sectional: '',
           Target: '1 face',
           Trajectory: '',
         },

rust/kcl-lib/src/std/extrude.rs

@@ -205,6 +205,21 @@ pub(crate) async fn do_post_extrude<'a>(
         vec![]
     };
 
+    // Face filtering attempt in order to resolve https://github.com/KittyCAD/modeling-app/issues/5328
+    // In case of a sectional sweep, empirically it looks that the first n faces that are yielded from the sweep
+    // are the ones that work with GetOppositeEdge and GetNextAdjacentEdge, aka the n sides in the sweep.
+    // So here we're figuring out that n number as yielded_sides_count here,
+    // making sure that circle() calls count but close() don't (no length)
+    let yielded_sides_count = sketch
+        .paths
+        .iter()
+        .filter(|p| {
+            let is_circle = matches!(p, Path::Circle { .. });
+            let has_length = p.get_base().from != p.get_base().to;
+            return is_circle || has_length;
+        })
+        .count();
+
     for (curve_id, face_id) in face_infos
         .iter()
         .filter(|face_info| face_info.cap == ExtrusionFaceCapType::None)
@@ -215,6 +230,7 @@ pub(crate) async fn do_post_extrude<'a>(
                 None
             }
         })
+        .take(yielded_sides_count)
     {
         // Batch these commands, because the Rust code doesn't actually care about the outcome.
         // So, there's no need to await them.

src/lang/modifyAst.ts

@@ -515,30 +515,54 @@ export function addShell({
   }
 }
 
-export function addSweep(
+export function addSweep({
-  node: Node<Program>,
+  node,
-  profileDeclarator: VariableDeclarator,
+  targetDeclarator,
-  pathDeclarator: VariableDeclarator
+  trajectoryDeclarator,
-): {
+  sectional,
+  variableName,
+  insertIndex,
+}: {
+  node: Node<Program>
+  targetDeclarator: VariableDeclarator
+  trajectoryDeclarator: VariableDeclarator
+  sectional: boolean
+  variableName?: string
+  insertIndex?: number
+}): {
   modifiedAst: Node<Program>
   pathToNode: PathToNode
 } {
   const modifiedAst = structuredClone(node)
-  const name = findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SWEEP)
+  const name =
-  const sweep = createCallExpressionStdLibKw(
+    variableName ?? findUniqueName(node, KCL_DEFAULT_CONSTANT_PREFIXES.SWEEP)
+  const call = createCallExpressionStdLibKw(
     'sweep',
-    createIdentifier(profileDeclarator.id.name),
+    createIdentifier(targetDeclarator.id.name),
-    [createLabeledArg('path', createIdentifier(pathDeclarator.id.name))]
+    [
+      createLabeledArg('path', createIdentifier(trajectoryDeclarator.id.name)),
+      createLabeledArg('sectional', createLiteral(sectional)),
+    ]
   )
-  const declaration = createVariableDeclaration(name, sweep)
+  const variable = createVariableDeclaration(name, call)
-  modifiedAst.body.push(declaration)
+  const insertAt =
+    insertIndex !== undefined
+      ? insertIndex
+      : modifiedAst.body.length
+      ? modifiedAst.body.length
+      : 0
+
+  modifiedAst.body.length
+    ? modifiedAst.body.splice(insertAt, 0, variable)
+    : modifiedAst.body.push(variable)
+  const argIndex = 0
   const pathToNode: PathToNode = [
     ['body', ''],
-    [modifiedAst.body.length - 1, 'index'],
+    [insertAt, 'index'],
     ['declaration', 'VariableDeclaration'],
     ['init', 'VariableDeclarator'],
     ['arguments', 'CallExpressionKw'],
-    [0, ARG_INDEX_FIELD],
+    [argIndex, ARG_INDEX_FIELD],
     ['arg', LABELED_ARG_FIELD],
   ]
 

src/lib/commandBarConfigs/modelingCommandConfig.ts

@@ -47,8 +47,12 @@ export type ModelingCommandSchema = {
     distance: KclCommandValue
   }
   Sweep: {
+    // Enables editing workflow
+    nodeToEdit?: PathToNode
+    // Arguments
     target: Selections
     trajectory: Selections
+    sectional: boolean
   }
   Loft: {
     selection: Selections
@@ -345,22 +349,40 @@ export const modelingMachineCommandConfig: StateMachineCommandSetConfig<
     description:
       'Create a 3D body by moving a sketch region along an arbitrary path.',
     icon: 'sweep',
-    needsReview: false,
+    needsReview: true,
     args: {
+      nodeToEdit: {
+        description:
+          'Path to the node in the AST to edit. Never shown to the user.',
+        skip: true,
+        inputType: 'text',
+        required: false,
+      },
       target: {
         inputType: 'selection',
         selectionTypes: ['solid2d'],
         required: true,
         skip: true,
         multiple: false,
+        hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
       },
       trajectory: {
         inputType: 'selection',
-        selectionTypes: ['segment', 'path'],
+        selectionTypes: ['segment'],
         required: true,
-        skip: false,
+        skip: true,
         multiple: false,
         validation: sweepValidator,
+        hidden: (context) => Boolean(context.argumentsToSubmit.nodeToEdit),
+      },
+      sectional: {
+        inputType: 'options',
+        required: true,
+        options: [
+          { name: 'False', value: false },
+          { name: 'True', value: true },
+        ],
+        // No validation possible here until we have rollback
       },
     },
   },

src/lib/operations.ts

@@ -311,6 +311,143 @@ const prepareToEditOffsetPlane: PrepareToEditCallback = async ({
   }
 }
 
+const prepareToEditSweep: PrepareToEditCallback = async ({
+  artifact,
+  operation,
+}) => {
+  const baseCommand = {
+    name: 'Sweep',
+    groupId: 'modeling',
+  }
+  if (
+    operation.type !== 'StdLibCall' ||
+    !operation.labeledArgs ||
+    !operation.unlabeledArg ||
+    !('sectional' in operation.labeledArgs) ||
+    !operation.labeledArgs.sectional
+  ) {
+    return baseCommand
+  }
+  if (!artifact || !('pathId' in artifact) || operation.type !== 'StdLibCall') {
+    return baseCommand
+  }
+
+  // We have to go a little roundabout to get from the original artifact
+  // to the solid2DId that we need to pass to the Sweep command, just like Extrude.
+  const pathArtifact = getArtifactOfTypes(
+    {
+      key: artifact.pathId,
+      types: ['path'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (
+    err(pathArtifact) ||
+    pathArtifact.type !== 'path' ||
+    !pathArtifact.solid2dId
+  ) {
+    return baseCommand
+  }
+
+  const targetArtifact = getArtifactOfTypes(
+    {
+      key: pathArtifact.solid2dId,
+      types: ['solid2d'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(targetArtifact) || targetArtifact.type !== 'solid2d') {
+    return baseCommand
+  }
+
+  const target = {
+    graphSelections: [
+      {
+        artifact: targetArtifact,
+        codeRef: pathArtifact.codeRef,
+      },
+    ],
+    otherSelections: [],
+  }
+
+  // Same roundabout but twice for 'path' aka trajectory: sketch -> path -> segment
+  if (!('path' in operation.labeledArgs) || !operation.labeledArgs.path) {
+    return baseCommand
+  }
+
+  if (operation.labeledArgs.path.value.type !== 'Sketch') {
+    return baseCommand
+  }
+
+  const trajectoryPathArtifact = getArtifactOfTypes(
+    {
+      key: operation.labeledArgs.path.value.value.artifactId,
+      types: ['path'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(trajectoryPathArtifact) || trajectoryPathArtifact.type !== 'path') {
+    return baseCommand
+  }
+
+  const trajectoryArtifact = getArtifactOfTypes(
+    {
+      key: trajectoryPathArtifact.segIds[0],
+      types: ['segment'],
+    },
+    engineCommandManager.artifactGraph
+  )
+
+  if (err(trajectoryArtifact) || trajectoryArtifact.type !== 'segment') {
+    return baseCommand
+  }
+
+  const trajectory = {
+    graphSelections: [
+      {
+        artifact: trajectoryArtifact,
+        codeRef: trajectoryArtifact.codeRef,
+      },
+    ],
+    otherSelections: [],
+  }
+
+  // sectional options boolean arg
+  if (
+    !('sectional' in operation.labeledArgs) ||
+    !operation.labeledArgs.sectional
+  ) {
+    return baseCommand
+  }
+
+  const sectional =
+    codeManager.code.slice(
+      operation.labeledArgs.sectional.sourceRange[0],
+      operation.labeledArgs.sectional.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['Sweep'] = {
+    target: target,
+    trajectory,
+    sectional,
+    nodeToEdit: getNodePathFromSourceRange(
+      kclManager.ast,
+      sourceRangeFromRust(operation.sourceRange)
+    ),
+  }
+
+  return {
+    ...baseCommand,
+    argDefaultValues,
+  }
+}
+
 const prepareToEditHelix: PrepareToEditCallback = async ({ operation }) => {
   const baseCommand = {
     name: 'Helix',
@@ -511,6 +648,7 @@ export const stdLibMap: Record<string, StdLibCallInfo> = {
   sweep: {
     label: 'Sweep',
     icon: 'sweep',
+    prepareToEdit: prepareToEditSweep,
     supportsAppearance: true,
   },
 }

src/machines/modelingMachine.ts

@@ -78,7 +78,7 @@ import {
 import { ModelingCommandSchema } from 'lib/commandBarConfigs/modelingCommandConfig'
 import { err, reportRejection, trap } from 'lib/trap'
 import { DefaultPlaneStr } from 'lib/planes'
-import { uuidv4 } from 'lib/utils'
+import { isArray, uuidv4 } from 'lib/utils'
 import { Coords2d } from 'lang/std/sketch'
 import { deleteSegment } from 'clientSideScene/ClientSideSceneComp'
 import toast from 'react-hot-toast'
@@ -97,6 +97,7 @@ import { createProfileStartHandle } from 'clientSideScene/segments'
 import { DRAFT_POINT } from 'clientSideScene/sceneInfra'
 import { setAppearance } from 'lang/modifyAst/setAppearance'
 import { DRAFT_DASHED_LINE } from 'clientSideScene/sceneEntities'
+import { updateModelingState } from 'lang/modelingWorkflows'
 
 export const MODELING_PERSIST_KEY = 'MODELING_PERSIST_KEY'
 
@@ -1988,55 +1989,88 @@ export const modelingMachine = setup({
         if (!input) return new Error('No input provided')
         // Extract inputs
         const ast = kclManager.ast
-        const { target, trajectory } = input
+        const { target, trajectory, sectional, nodeToEdit } = input
+        let variableName: string | undefined = undefined
+        let insertIndex: number | undefined = undefined
 
-        // Find the profile declaration
+        // If this is an edit flow, first we're going to remove the old one
+        if (nodeToEdit !== undefined && typeof nodeToEdit[1][0] === 'number') {
+          // Extract the plane name from the node to edit
+          const variableNode = getNodeFromPath<VariableDeclaration>(
+            ast,
+            nodeToEdit,
+            'VariableDeclaration'
+          )
+
+          if (err(variableNode)) {
+            console.error('Error extracting name')
+          } else {
+            variableName = variableNode.node.declaration.id.name
+          }
+
+          // Removing the old statement
+          const newBody = [...ast.body]
+          newBody.splice(nodeToEdit[1][0], 1)
+          ast.body = newBody
+          insertIndex = nodeToEdit[1][0]
+        }
+
+        // Find the target declaration
         const targetNodePath = getNodePathFromSourceRange(
           ast,
           target.graphSelections[0].codeRef.range
         )
-        const targetNode = getNodeFromPath<VariableDeclarator>(
+        // Gotchas, not sure why
-          ast,
+        // - it seems like in some cases we get a list on edit, especially the state that e2e hits
-          targetNodePath,
+        // - looking for a VariableDeclaration seems more robust than VariableDeclarator
-          'VariableDeclarator'
+        const targetNode = getNodeFromPath<
-        )
+          VariableDeclaration | VariableDeclaration[]
+        >(ast, targetNodePath, 'VariableDeclaration')
         if (err(targetNode)) {
           return new Error("Couldn't parse profile selection")
         }
-        const targetDeclarator = targetNode.node
 
-        // Find the path declaration
+        const targetDeclarator = isArray(targetNode.node)
+          ? targetNode.node[0].declaration
+          : targetNode.node.declaration
+
+        // Find the trajectory (or path) declaration
         const trajectoryNodePath = getNodePathFromSourceRange(
           ast,
           trajectory.graphSelections[0].codeRef.range
         )
-        const trajectoryNode = getNodeFromPath<VariableDeclarator>(
+        // Also looking for VariableDeclaration for consistency here
+        const trajectoryNode = getNodeFromPath<VariableDeclaration>(
           ast,
           trajectoryNodePath,
-          'VariableDeclarator'
+          'VariableDeclaration'
         )
         if (err(trajectoryNode)) {
           return new Error("Couldn't parse path selection")
         }
-        const trajectoryDeclarator = trajectoryNode.node
+
+        const trajectoryDeclarator = trajectoryNode.node.declaration
 
         // Perform the sweep
-        const sweepRes = addSweep(ast, targetDeclarator, trajectoryDeclarator)
+        const { modifiedAst, pathToNode } = addSweep({
-        const updateAstResult = await kclManager.updateAst(
+          node: ast,
-          sweepRes.modifiedAst,
+          targetDeclarator,
-          true,
+          trajectoryDeclarator,
+          sectional,
+          variableName,
+          insertIndex,
+        })
+        await updateModelingState(
+          modifiedAst,
           {
-            focusPath: [sweepRes.pathToNode],
+            kclManager,
+            editorManager,
+            codeManager,
+          },
+          {
+            focusPath: [pathToNode],
           }
         )
-
-        await codeManager.updateEditorWithAstAndWriteToFile(
-          updateAstResult.newAst
-        )
-
-        if (updateAstResult?.selections) {
-          editorManager.selectRange(updateAstResult?.selections)
-        }
       }
     ),
     loftAstMod: fromPromise(