From 3d042b0375a96b94ecd27397561922edb1c4dec4 Mon Sep 17 00:00:00 2001 From: max Date: Fri, 3 Oct 2025 20:02:07 +0200 Subject: [PATCH 1/7] Handle edgeCut selection in getSweepArtifactFromSelection Added logic to support 'edgeCut' artifact type in getSweepArtifactFromSelection. The function now retrieves the associated sweep artifact by resolving the consumed edge, handling both 'segment' and 'sweepEdge' cases. --- src/lang/std/artifactGraph.ts | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/lang/std/artifactGraph.ts b/src/lang/std/artifactGraph.ts index 8a105eec005..5bb3ed7d15d 100644 --- a/src/lang/std/artifactGraph.ts +++ b/src/lang/std/artifactGraph.ts @@ -515,6 +515,35 @@ export function getSweepArtifactFromSelection( ) if (err(_artifact)) return _artifact sweepArtifact = _artifact + } else if (selection.artifact?.type === 'edgeCut') { + // Handle edgeCut by getting its consumed edge (segment or sweepEdge) + const segOrEdge = getArtifactOfTypes( + { + key: selection.artifact.consumedEdgeId, + types: ['segment', 'sweepEdge'], + }, + artifactGraph + ) + if (err(segOrEdge)) return segOrEdge + + // If it's a segment, traverse through path to get sweep + if (segOrEdge.type === 'segment') { + const path = getArtifactOfTypes( + { key: segOrEdge.pathId, types: ['path'] }, + artifactGraph + ) + if (err(path)) return path + if (!path.sweepId) return new Error('Path does not have a sweepId') + return getArtifactOfTypes( + { key: path.sweepId, types: ['sweep'] }, + artifactGraph + ) + } + // Otherwise it's a sweepEdge, get sweep directly + return getArtifactOfTypes( + { key: segOrEdge.sweepId, types: ['sweep'] }, + artifactGraph + ) } if (!sweepArtifact) return new Error('No sweep artifact found') From 53970fed12dcb7b0a94c47807f118d0a15885ac3 Mon Sep 17 00:00:00 2001 From: max Date: Fri, 3 Oct 2025 20:02:55 +0200 Subject: [PATCH 2/7] Add function to coerce selections to body artifacts Introduces coerceSelectionsToBody to convert selections containing faces or edges to their parent body artifacts. This utility helps commands that require body-level selections by ensuring only body artifacts are returned, handling various artifact types and avoiding duplicates. --- src/lang/std/artifactGraph.ts | 82 ++++++++++++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 1 deletion(-) diff --git a/src/lang/std/artifactGraph.ts b/src/lang/std/artifactGraph.ts index 5bb3ed7d15d..d124a02cba1 100644 --- a/src/lang/std/artifactGraph.ts +++ b/src/lang/std/artifactGraph.ts @@ -29,7 +29,7 @@ import type { SweepEdge, WallArtifact, } from '@src/lang/wasm' -import type { Selection } from '@src/lib/selections' +import type { Selection, Selections } from '@src/lib/selections' import { err } from '@src/lib/trap' export type { Artifact, ArtifactId, SegmentArtifact } from '@src/lang/wasm' @@ -868,3 +868,83 @@ export function getFaceCodeRef( } return null } + +/** + * Coerce selections that may contain faces or edges to their parent body (sweep/compositeSolid). + * This is useful for commands that only work with bodies, but users may have faces or edges selected. + * + * @param selections - The selections to coerce + * @param artifactGraph - The artifact graph to use for lookups + * @returns A new Selections object with only body artifacts, or an Error if coercion fails + */ +export function coerceSelectionsToBody( + selections: Selections, + artifactGraph: ArtifactGraph +): Selections | Error { + const bodySelections: Selection[] = [] + const seenBodyIds = new Set() + + for (const selection of selections.graphSelections) { + if (!selection.artifact) { + continue + } + + let bodyArtifact: Artifact | null = null + let bodyCodeRef: CodeRef | null = null + + // If it's already a body type, use it directly + if ( + selection.artifact.type === 'sweep' || + selection.artifact.type === 'compositeSolid' || + selection.artifact.type === 'path' + ) { + bodyArtifact = selection.artifact + bodyCodeRef = selection.codeRef + } else { + // Get the parent body (sweep) from faces, edges, or edgeCuts + // getSweepArtifactFromSelection handles all common types: sweepEdge, segment, wall, cap, edgeCut + const sweep = getSweepArtifactFromSelection(selection, artifactGraph) + + if (err(sweep)) { + return new Error( + `Unable to find parent body for selected artifact: ${selection.artifact.type}` + ) + } + + // Prefer the path over the sweep for the final selection + // The engine selects the path (not the sweep) when the object filter is active, + // so we follow the same convention for consistency + const sweepArtifact = sweep as Artifact + if ('pathId' in sweepArtifact && sweepArtifact.pathId) { + const path = getArtifactOfTypes( + { key: sweepArtifact.pathId, types: ['path'] }, + artifactGraph + ) + if (!err(path)) { + bodyArtifact = path + bodyCodeRef = path.codeRef + } else { + bodyArtifact = sweepArtifact + bodyCodeRef = sweep.codeRef + } + } else { + bodyArtifact = sweepArtifact + bodyCodeRef = sweep.codeRef + } + } + + // Add to the result, avoiding duplicates + if (bodyArtifact && bodyCodeRef && !seenBodyIds.has(bodyArtifact.id)) { + seenBodyIds.add(bodyArtifact.id) + bodySelections.push({ + artifact: bodyArtifact, + codeRef: bodyCodeRef, + }) + } + } + + return { + graphSelections: bodySelections, + otherSelections: selections.otherSelections, + } +} From 8d09e80301c4d711b1849e9648ff3461456c3409 Mon Sep 17 00:00:00 2001 From: max Date: Fri, 3 Oct 2025 20:26:27 +0200 Subject: [PATCH 3/7] Coerce selections to bodies for body-only arguments Adds logic to automatically coerce selections to body types (path, sweep, compositeSolid) when the argument only accepts bodies. Updates selection filter handling to ensure correct selection state after coercion, improving support for body-only commands in the command bar. --- .../CommandBarSelectionMixedInput.tsx | 60 +++++++++++++++++-- 1 file changed, 56 insertions(+), 4 deletions(-) diff --git a/src/components/CommandBar/CommandBarSelectionMixedInput.tsx b/src/components/CommandBar/CommandBarSelectionMixedInput.tsx index f17ee5a8f14..3a8cb0a76a9 100644 --- a/src/components/CommandBar/CommandBarSelectionMixedInput.tsx +++ b/src/components/CommandBar/CommandBarSelectionMixedInput.tsx @@ -10,6 +10,8 @@ import { } from '@src/lib/selections' import { kclManager } from '@src/lib/singletons' import { commandBarActor, useCommandBarState } from '@src/lib/singletons' +import { coerceSelectionsToBody } from '@src/lang/std/artifactGraph' +import { err } from '@src/lib/trap' const selectionSelector = (snapshot: any) => snapshot?.context.selectionRanges @@ -26,11 +28,52 @@ export default function CommandBarSelectionMixedInput({ const commandBarState = useCommandBarState() const [hasSubmitted, setHasSubmitted] = useState(false) const [hasAutoSkipped, setHasAutoSkipped] = useState(false) + const [hasCoercedSelections, setHasCoercedSelections] = useState(false) const selection: Selections = useSelector(arg.machineActor, selectionSelector) const selectionsByType = useMemo(() => { return getSelectionCountByType(selection) }, [selection]) + + // Coerce selections to bodies if this argument requires bodies + useEffect(() => { + // Only run once per component mount + if (hasCoercedSelections) return + + // Mark as attempted to prevent infinite loops + setHasCoercedSelections(true) + + if (!selection || selection.graphSelections.length === 0) return + + // Check if this argument only accepts body types (path, sweep, compositeSolid) + // These are the artifact types that represent 3D bodies/objects + const onlyAcceptsBodies = arg.selectionTypes?.every( + (type) => type === 'sweep' || type === 'compositeSolid' || type === 'path' + ) + + if (onlyAcceptsBodies && arg.machineActor) { + const coercedSelections = coerceSelectionsToBody( + selection, + kclManager.artifactGraph + ) + + if (!err(coercedSelections)) { + // Immediately update the modeling machine state with coerced selection + // This needs to happen BEFORE the selection filter is applied + if (arg.machineActor) { + arg.machineActor.send({ + type: 'Set selection', + data: { + selectionType: 'completeSelection', + selection: coercedSelections, + }, + }) + } + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- Only run on mount + }, []) + const isArgRequired = arg.required instanceof Function ? arg.required(commandBarState.context) @@ -67,11 +110,20 @@ export default function CommandBarSelectionMixedInput({ }, [arg.name]) // Set selection filter if needed, and reset it when the component unmounts + // This runs after coercion completes and updates the selection useEffect(() => { - arg.selectionFilter && kclManager.setSelectionFilter(arg.selectionFilter) - return () => kclManager.defaultSelectionFilter(selection) - // eslint-disable-next-line react-hooks/exhaustive-deps -- TODO: blanket-ignored fix me! - }, [arg.selectionFilter]) + if (arg.selectionFilter && hasCoercedSelections) { + // Pass the current selection to restore it after applying the filter + // This is critical for body-only commands where we've coerced face/edge selections to bodies + kclManager.setSelectionFilter(arg.selectionFilter, selection) + } + return () => { + if (arg.selectionFilter && hasCoercedSelections) { + kclManager.defaultSelectionFilter(selection) + } + } + // eslint-disable-next-line react-hooks/exhaustive-deps -- Need to react to selection changes after coercion + }, [arg.selectionFilter, selection, hasCoercedSelections]) // Watch for outside teardowns of this component // (such as clicking another argument in the command palette header) From 8eae2a493d6c0103b574244ed5e8151360cc2552 Mon Sep 17 00:00:00 2001 From: max Date: Fri, 3 Oct 2025 20:42:30 +0200 Subject: [PATCH 4/7] Update setSelectionFilter to accept selectionsToRestore Added an optional selectionsToRestore parameter to setSelectionFilter in KclManager and passed it to setSelectionFilter implementation. This allows restoring selections when setting the filter. --- src/lang/KclSingleton.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lang/KclSingleton.ts b/src/lang/KclSingleton.ts index a7fb87d70af..20aa4191bfe 100644 --- a/src/lang/KclSingleton.ts +++ b/src/lang/KclSingleton.ts @@ -790,8 +790,8 @@ export class KclManager extends EventTarget { setSelectionFilterToDefault(this.engineCommandManager, selectionsToRestore) } /** TODO: this function is hiding unawaited asynchronous work */ - setSelectionFilter(filter: EntityType[]) { - setSelectionFilter(filter, this.engineCommandManager) + setSelectionFilter(filter: EntityType[], selectionsToRestore?: Selections) { + setSelectionFilter(filter, this.engineCommandManager, selectionsToRestore) } // Determines if there is no KCL code which means it is executing a blank KCL file From 2e70319880a416d9b0f96ccaf68e36576d76adec Mon Sep 17 00:00:00 2001 From: max Date: Fri, 3 Oct 2025 22:00:03 +0200 Subject: [PATCH 5/7] Update Selection types import source --- src/lang/std/artifactGraph.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lang/std/artifactGraph.ts b/src/lang/std/artifactGraph.ts index dad3e40eb61..627ccb22c74 100644 --- a/src/lang/std/artifactGraph.ts +++ b/src/lang/std/artifactGraph.ts @@ -29,8 +29,7 @@ import type { SweepEdge, WallArtifact, } from '@src/lang/wasm' -import type { Selections } from '@src/lib/selections' -import type { Selection } from '@src/machines/modelingSharedTypes' +import type { Selection, Selections } from '@src/machines/modelingSharedTypes' import { err } from '@src/lib/trap' export type { Artifact, ArtifactId, SegmentArtifact } from '@src/lang/wasm' From 00ab3749459fb142cca54dd580e905d173832a21 Mon Sep 17 00:00:00 2001 From: max Date: Sat, 4 Oct 2025 14:32:35 +0200 Subject: [PATCH 6/7] Add tests for artifactGraph selection utilities Introduces unit tests for getSweepArtifactFromSelection and coerceSelectionsToBody functions, verifying correct artifact resolution and selection coercion behavior in artifactGraph. --- src/lang/std/artifactGraph.test.ts | 185 +++++++++++++++++++++++++++++ 1 file changed, 185 insertions(+) create mode 100644 src/lang/std/artifactGraph.test.ts diff --git a/src/lang/std/artifactGraph.test.ts b/src/lang/std/artifactGraph.test.ts new file mode 100644 index 00000000000..6cd7c8d0914 --- /dev/null +++ b/src/lang/std/artifactGraph.test.ts @@ -0,0 +1,185 @@ +import { describe, it, expect } from 'vitest' +import { + coerceSelectionsToBody, + getSweepArtifactFromSelection, + type Artifact, +} from './artifactGraph' +import type { ArtifactGraph } from '@src/lang/wasm' +import type { Selections, Selection } from '@src/machines/modelingSharedTypes' + +describe('getSweepArtifactFromSelection', () => { + it('should return sweep from edgeCut selection', () => { + const artifactGraph: ArtifactGraph = new Map() + + // Create path -> sweep -> segment -> edgeCut chain + const path: Artifact = { + type: 'path', + id: 'path-1', + codeRef: { range: [0, 0, 0], pathToNode: [], nodePath: { steps: [] } }, + planeId: 'plane-1', + segIds: ['segment-1'], + sweepId: 'sweep-1', + } + + const sweep: Artifact = { + type: 'sweep', + id: 'sweep-1', + codeRef: { + range: [0, 0, 0], + pathToNode: [], + nodePath: { steps: [] }, + }, + pathId: 'path-1', + subType: 'extrusion', + surfaceIds: [], + edgeIds: [], + } + + const segment: Artifact = { + type: 'segment', + id: 'segment-1', + pathId: 'path-1', + edgeIds: [], + commonSurfaceIds: [], + codeRef: { + range: [0, 0, 0], + pathToNode: [], + nodePath: { steps: [] }, + }, + } + + const edgeCut: Artifact = { + type: 'edgeCut', + id: 'edge-cut-1', + consumedEdgeId: 'segment-1', + subType: 'chamfer', + edgeIds: [], + codeRef: { + range: [0, 0, 0], + pathToNode: [], + nodePath: { steps: [] }, + }, + } + + artifactGraph.set('path-1', path) + artifactGraph.set('sweep-1', sweep) + artifactGraph.set('segment-1', segment) + artifactGraph.set('edge-cut-1', edgeCut) + + const selection: Selection = { + artifact: edgeCut, + codeRef: { range: [0, 0, 0], pathToNode: [] }, + } + + const result = getSweepArtifactFromSelection(selection, artifactGraph) + + expect(result).not.toBeInstanceOf(Error) + if (!(result instanceof Error)) { + expect('type' in result ? result.type : undefined).toBe('sweep') + expect(result.id).toBe('sweep-1') + } + }) +}) + +describe('coerceSelectionsToBody', () => { + it('should pass through path artifact unchanged', () => { + const artifactGraph: ArtifactGraph = new Map() + + const path: Artifact = { + type: 'path', + id: 'path-1', + codeRef: { range: [0, 100, 0], pathToNode: [], nodePath: { steps: [] } }, + planeId: 'plane-1', + segIds: [], + } + artifactGraph.set('path-1', path) + + const selections: Selections = { + graphSelections: [ + { + artifact: path, + codeRef: { range: [0, 100, 0], pathToNode: [] }, + }, + ], + otherSelections: [], + } + + const result = coerceSelectionsToBody(selections, artifactGraph) + + expect(result).not.toBeInstanceOf(Error) + if (!(result instanceof Error)) { + expect(result.graphSelections).toHaveLength(1) + expect(result.graphSelections[0].artifact?.type).toBe('path') + expect(result.graphSelections[0].artifact?.id).toBe('path-1') + } + }) + + it('should coerce edgeCut selection to parent path', () => { + const artifactGraph: ArtifactGraph = new Map() + + const path: Artifact = { + type: 'path', + id: 'path-1', + codeRef: { range: [0, 100, 0], pathToNode: [], nodePath: { steps: [] } }, + planeId: 'plane-1', + segIds: ['segment-1'], + sweepId: 'sweep-1', + } + + const sweep: Artifact = { + type: 'sweep', + id: 'sweep-1', + codeRef: { + range: [100, 200, 0], + pathToNode: [], + nodePath: { steps: [] }, + }, + pathId: 'path-1', + subType: 'extrusion', + surfaceIds: [], + edgeIds: [], + } + + const segment: Artifact = { + type: 'segment', + id: 'segment-1', + pathId: 'path-1', + edgeIds: [], + commonSurfaceIds: [], + codeRef: { range: [10, 20, 0], pathToNode: [], nodePath: { steps: [] } }, + } + + const edgeCut: Artifact = { + type: 'edgeCut', + id: 'edge-cut-1', + consumedEdgeId: 'segment-1', + subType: 'chamfer', + edgeIds: [], + codeRef: { range: [90, 95, 0], pathToNode: [], nodePath: { steps: [] } }, + } + + artifactGraph.set('path-1', path) + artifactGraph.set('sweep-1', sweep) + artifactGraph.set('segment-1', segment) + artifactGraph.set('edge-cut-1', edgeCut) + + const selections: Selections = { + graphSelections: [ + { + artifact: edgeCut, + codeRef: { range: [90, 95, 0], pathToNode: [] }, + }, + ], + otherSelections: [], + } + + const result = coerceSelectionsToBody(selections, artifactGraph) + + expect(result).not.toBeInstanceOf(Error) + if (!(result instanceof Error)) { + expect(result.graphSelections).toHaveLength(1) + expect(result.graphSelections[0].artifact?.type).toBe('path') + expect(result.graphSelections[0].artifact?.id).toBe('path-1') + } + }) +}) From bff8495fc3da9d0576c2d1b3d46af50ae5d63aa9 Mon Sep 17 00:00:00 2001 From: max Date: Sat, 4 Oct 2025 15:43:11 +0200 Subject: [PATCH 7/7] Pass handleSelectionBatch to setSelectionFilter Updates setSelectionFilter to explicitly pass handleSelectionBatchFn as an argument, ensuring the correct function is used for selection batch handling. --- src/lang/KclSingleton.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/lang/KclSingleton.ts b/src/lang/KclSingleton.ts index 211cdcd69e8..dcc75e24659 100644 --- a/src/lang/KclSingleton.ts +++ b/src/lang/KclSingleton.ts @@ -51,7 +51,7 @@ import type { PlaneVisibilityMap, Selections, } from '@src/machines/modelingSharedTypes' -import { type handleSelectionBatch as handleSelectionBatchFn } from '@src/lib/selections' +import { handleSelectionBatch as handleSelectionBatchFn } from '@src/lib/selections' interface ExecuteArgs { ast?: Node @@ -793,7 +793,12 @@ export class KclManager extends EventTarget { } /** TODO: this function is hiding unawaited asynchronous work */ setSelectionFilter(filter: EntityType[], selectionsToRestore?: Selections) { - setSelectionFilter(filter, this.engineCommandManager, selectionsToRestore) + setSelectionFilter( + filter, + this.engineCommandManager, + selectionsToRestore, + handleSelectionBatchFn + ) } // Determines if there is no KCL code which means it is executing a blank KCL file