Skip to content

Artifact graph: add link between Sweep::Sweep and trajectory Path or Helix, add Helix from Cylinder to Artifact Graph #9724

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
franknoirot wants to merge 6 commits into
base: main
Choose a base branch
from
+171 −60
Open

Artifact graph: add link between Sweep::Sweep and trajectory Path or Helix, add Helix from Cylinder to Artifact Graph #9724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
25696f0
Add `sweep_id` field to Helix
franknoirot Jan 16, 2026
f9d18d5
Add helices created from cylinders to artifact graph
franknoirot Jan 16, 2026
b050094
Break out Sweep match branch so we can use trajectory
franknoirot Jan 16, 2026
a63ea57
Update sim test output
franknoirot Jan 16, 2026
e6c20f9
Add helix -> sweep parent-child relationship in mermaid tests
franknoirot Jan 17, 2026
da0001e
Update mermaid output after adding parent-child relationship
franknoirot Jan 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 74 additions & 4 deletions rust/kcl-lib/src/execution/artifact.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ pub struct Helix {
/// add axes to the graph.
pub axis_id: Option<ArtifactId>,
pub code_ref: CodeRef,
pub sweep_id: Option<ArtifactId>,
}

#[derive(Debug, Clone, Serialize, PartialEq, ts_rs::TS)]
Expand Down Expand Up @@ -459,7 +460,7 @@ impl Artifact {
Artifact::SweepEdge(_) => Some(new),
Artifact::EdgeCut(a) => a.merge(new),
Artifact::EdgeCutEdge(_) => Some(new),
Artifact::Helix(_) => Some(new),
Artifact::Helix(a) => a.merge(new),
}
}
}
Expand Down Expand Up @@ -566,6 +567,18 @@ impl EdgeCut {
}
}

impl Helix {
fn merge(&mut self, new: Artifact) -> Option<Artifact> {
let Artifact::Helix(new) = new else {
return Some(new);
};
merge_opt_id(&mut self.axis_id, new.axis_id);
merge_opt_id(&mut self.sweep_id, new.sweep_id);

None
}
}

#[derive(Debug, Clone, Default, PartialEq, Serialize, ts_rs::TS)]
#[ts(export_to = "Artifact.ts")]
#[serde(rename_all = "camelCase")]
Expand Down Expand Up @@ -768,6 +781,7 @@ fn merge_ids(base: &mut Vec<ArtifactId>, new: Vec<ArtifactId>) {
}
}

/// Merge optional Artifact ID
fn merge_opt_id(base: &mut Option<ArtifactId>, new: Option<ArtifactId>) {
// Always use the new one, even if it clears it.
*base = new;
Expand Down Expand Up @@ -1024,8 +1038,7 @@ fn artifacts_to_update(
| ModelingCmd::TwistExtrude(kcmc::TwistExtrude { target, .. })
| ModelingCmd::Revolve(kcmc::Revolve { target, .. })
| ModelingCmd::RevolveAboutEdge(kcmc::RevolveAboutEdge { target, .. })
| ModelingCmd::ExtrudeToReference(kcmc::ExtrudeToReference { target, .. })
| ModelingCmd::Sweep(kcmc::Sweep { target, .. }) => {
| ModelingCmd::ExtrudeToReference(kcmc::ExtrudeToReference { target, .. }) => {
// Determine the resulting method from the specific command, if provided
let method = match cmd {
ModelingCmd::Extrude(kcmc::Extrude { extrude_method, .. }) => *extrude_method,
Expand All @@ -1046,7 +1059,6 @@ fn artifacts_to_update(
ModelingCmd::TwistExtrude(_) => SweepSubType::ExtrusionTwist,
ModelingCmd::Revolve(_) => SweepSubType::Revolve,
ModelingCmd::RevolveAboutEdge(_) => SweepSubType::RevolveAboutEdge,
ModelingCmd::Sweep(_) => SweepSubType::Sweep,
_ => internal_error!(range, "Sweep-like command variant not handled: id={id:?}, cmd={cmd:?}",),
};
let mut return_arr = Vec::new();
Expand Down Expand Up @@ -1075,6 +1087,52 @@ fn artifacts_to_update(
}
return Ok(return_arr);
}
ModelingCmd::Sweep(kcmc::Sweep { target, trajectory, .. }) => {
// Determine the resulting method from the specific command, if provided
let method = kittycad_modeling_cmds::shared::ExtrudeMethod::Merge;
let sub_type = SweepSubType::Sweep;
let mut return_arr = Vec::new();
let target = ArtifactId::from(target);
let trajectory = ArtifactId::from(trajectory);
return_arr.push(Artifact::Sweep(Sweep {
id,
sub_type,
path_id: target,
surface_ids: Vec::new(),
edge_ids: Vec::new(),
code_ref,
method,
}));
let path = artifacts.get(&target);
if let Some(Artifact::Path(path)) = path {
let mut new_path = path.clone();
new_path.sweep_id = Some(id);
return_arr.push(Artifact::Path(new_path));
if let Some(inner_path_id) = path.inner_path_id
&& let Some(inner_path_artifact) = artifacts.get(&inner_path_id)
&& let Artifact::Path(mut inner_path_artifact) = inner_path_artifact.clone()
{
inner_path_artifact.sweep_id = Some(id);
return_arr.push(Artifact::Path(inner_path_artifact))
}
}
if let Some(trajectory_artifact) = artifacts.get(&trajectory) {
match trajectory_artifact {
Artifact::Path(path) => {
let mut new_path = path.clone();
new_path.sweep_id = Some(id);
Copy link
Contributor

@jtran jtran Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to do this? It kind of overloads the path's sweep_id to mean both that the path was swept and that it was swept along.

Copy link
Contributor Author

@franknoirot franknoirot Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay so would it be more prudent to add a new field that indicates the relationship is a trajectory specifically? The name sweep_id was already ambiguous to indicate a path was used as th profile to be swept imo

Copy link
Contributor

@jtran jtran Jan 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's more accurately modeling what's happening if we used a new field, yeah. But it all depends on how it's used, which I don't know.

return_arr.push(Artifact::Path(new_path));
}
Artifact::Helix(helix) => {
let mut new_helix = helix.clone();
new_helix.sweep_id = Some(id);
return_arr.push(Artifact::Helix(new_helix));
}
_ => {}
}
};
return Ok(return_arr);
}
ModelingCmd::Loft(loft_cmd) => {
let Some(OkModelingCmdResponse::Loft(_)) = response else {
return Ok(Vec::new());
Expand Down Expand Up @@ -1382,11 +1440,22 @@ fn artifacts_to_update(
}
return Ok(return_arr);
}
ModelingCmd::EntityMakeHelix(cmd) => {
let cylinder_id = ArtifactId::new(cmd.cylinder_id);
let return_arr = vec![Artifact::Helix(Helix {
id,
axis_id: Some(cylinder_id),
code_ref,
sweep_id: None,
})];
return Ok(return_arr);
}
ModelingCmd::EntityMakeHelixFromParams(_) => {
let return_arr = vec![Artifact::Helix(Helix {
id,
axis_id: None,
code_ref,
sweep_id: None,
})];
return Ok(return_arr);
}
Expand All @@ -1396,6 +1465,7 @@ fn artifacts_to_update(
id,
axis_id: Some(edge_id),
code_ref,
sweep_id: None,
})];
// We could add the reverse graph edge connecting from the edge to
// the helix here, but it's not useful right now.
Expand Down
8 changes: 6 additions & 2 deletions rust/kcl-lib/src/execution/artifact/mermaid_tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,13 @@ impl Artifact {
// Note: Don't include these since they're parents: edge_cut_id.
vec![a.surface_id]
}
Artifact::Helix(_) => {
Artifact::Helix(a) => {
// Note: Don't include these since they're parents: axis_id.
Vec::new()
let mut ids = Vec::new();
if let Some(sweep_id) = a.sweep_id {
ids.push(sweep_id);
}
ids
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions rust/kcl-lib/tests/helix_ccw/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@ flowchart LR
%% face_code_ref=Missing NodePath
9["SweepEdge Opposite"]
10["SweepEdge Adjacent"]
11["Helix<br>[101, 195, 0]"]
%% [ProgramBodyItem { index: 0 }, VariableDeclarationDeclaration, VariableDeclarationInit, PipeBodyItem { index: 3 }]
1 --- 2
2 --- 3
2 --- 4
2 ---- 5
2 <--x 11
3 --- 6
3 x--> 7
3 --- 9
Expand Down
2 changes: 2 additions & 0 deletions rust/kcl-lib/tests/kcl_samples/bench/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2978,6 +2978,7 @@ flowchart LR
649 <--x 733
650 --- 651
650 --- 652
650 x---> 663
653 --- 654
653 <--x 734
654 --- 655
Expand Down Expand Up @@ -3072,6 +3073,7 @@ flowchart LR
687 <--x 735
688 --- 689
688 --- 690
688 x---> 701
691 --- 692
691 <--x 736
692 --- 693
Expand Down
1 change: 1 addition & 0 deletions rust/kcl-lib/tests/kcl_samples/coilover-assembly/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,7 @@ flowchart LR
110 --- 113
111 --- 112
111 --- 113
114 x--> 119
115 --- 116
116 --- 117
116 --- 118
Expand Down
1 change: 1 addition & 0 deletions rust/kcl-lib/tests/kcl_samples/cold-plate/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ flowchart LR
113 --- 118
113 --- 119
113 --- 120
113 x---> 128
121 --- 122
121 --- 125
121 <--x 158
Expand Down
6 changes: 6 additions & 0 deletions rust/kcl-lib/tests/kcl_samples/cpu-cooler/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1916,6 +1916,7 @@ flowchart LR
247 --- 251
247 --- 252
247 --- 253
247 x---> 261
254 --- 255
254 --- 258
254 <--x 767
Expand Down Expand Up @@ -1949,6 +1950,7 @@ flowchart LR
268 --- 275
268 --- 276
268 --- 277
268 x---> 285
278 --- 279
278 --- 282
278 <--x 769
Expand Down Expand Up @@ -2974,6 +2976,7 @@ flowchart LR
653 --- 661
653 --- 662
653 --- 663
653 x---> 680
664 --- 665
664 <--x 773
665 --- 666
Expand All @@ -2986,6 +2989,7 @@ flowchart LR
665 --- 673
665 --- 674
665 --- 675
665 x---> 689
676 --- 677
676 --- 686
676 <--x 774
Expand Down Expand Up @@ -3026,13 +3030,15 @@ flowchart LR
696 --- 699
696 --- 700
696 --- 701
696 x---> 713
702 --- 703
702 <--x 776
703 --- 704
703 --- 705
703 --- 706
703 --- 707
703 --- 708
703 x---> 722
709 --- 710
709 --- 719
709 <--x 777
Expand Down
4 changes: 4 additions & 0 deletions rust/kcl-lib/tests/kcl_samples/exhaust-manifold/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ flowchart LR
2 --- 5
2 --- 6
2 --- 7
2 x---> 15
8 --- 9
8 --- 12
9 --- 10
Expand Down Expand Up @@ -376,6 +377,7 @@ flowchart LR
22 --- 25
22 --- 26
22 --- 27
22 x---> 35
28 --- 29
28 --- 32
29 --- 30
Expand Down Expand Up @@ -403,6 +405,7 @@ flowchart LR
42 --- 45
42 --- 46
42 --- 47
42 x---> 55
48 --- 49
48 --- 52
49 --- 50
Expand Down Expand Up @@ -430,6 +433,7 @@ flowchart LR
62 --- 65
62 --- 66
62 --- 67
62 x---> 75
68 --- 69
68 --- 72
69 --- 70
Expand Down
4 changes: 4 additions & 0 deletions rust/kcl-lib/tests/kcl_samples/hand-trolley/artifact_graph_flowchart.snap.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1541,6 +1541,7 @@ flowchart LR
154 --- 158
154 --- 159
154 --- 160
154 x---> 165
161 --- 162
161 <--x 648
162 --- 163
Expand Down Expand Up @@ -1568,6 +1569,7 @@ flowchart LR
172 --- 179
172 --- 180
172 --- 181
172 x---> 186
182 --- 183
183 --- 184
183 --- 185
Expand Down Expand Up @@ -2029,6 +2031,7 @@ flowchart LR
359 --- 363
359 --- 364
359 --- 365
359 x---> 370
366 --- 367
366 <--x 653
367 --- 368
Expand Down Expand Up @@ -2056,6 +2059,7 @@ flowchart LR
377 --- 384
377 --- 385
377 --- 386
377 x---> 391
387 --- 388
388 --- 389
388 --- 390
Expand Down
Loading
Loading