Non-destructive, partial surface extrudes/revolves/sweeps etc by adamchalmers · Pull Request #1130 · KittyCAD/modeling-api

adamchalmers · 2026-03-12T15:28:10Z

This proposes a new way to surface extrude/revolve/sweep for sketch 2. Part of KittyCAD/modeling-app#10386

Background

Currently, the engine extrude/revolve/sweep/etc endpoints take an entire sketch as their target. That worked fine for sketch 1 solid extrudes (where the sketch is a closed profile) and for surface extrudes (where the sketch could be closed or open, either way, it extrudes each segment separately).

Problem

In sketch 2, the user might want to surface extrude only some segments in the sketch. Like in the below example, only k of the n segments are selected for extrusion.

Right now the API doesn't allow for this. Clients send the ID of the overall sketch, and the server extrudes the whole thing. There's no way to only surface extrude k of the n segments.

Solution

Instead of extruding an entire sketch (solid2D) at once, we instead take a list of segments and extrude each of those separately.

Here's a motivating example:

In this example, after extruding:

C becomes a single-surface body
D and E become a polysurface (D is extruded into one surface, E into another)
A and B become a single-body surface by default (because they're collinear) unless the user opts out by setting a keepSeams flag in KCL.

We want to:

Keep the old path segments A-E existing, unchanged (same IDs).
Create new bodies (Solid3Ds) from them.
- Frontend cannot predict the number of bodies in advance (because frontend doesn't know which segments intersect, or are collinear).
- So engine will need to tell the frontend how many bodies were created, and what their IDs are.
Get a mapping between segments and surfaces, so that tags can transfer.
- e.g. frontend must know that:
- segment A and B were absorbed into body 1 face 0
- segment C became body 2 face 0
- segment D became body 0 face 0
- segment E became body 0 face 1

Ideally we would not need any follow-up calls to query these bodies. Let's just send back all relevant information in the Extrude response.

Here's my proposed return for Extrude. I'm using YAML because it's more concise, but we'd actually be sending JSON. See the attachment for syntax highlighting, or copypastable here:

bodies:
  - id: body0
    surfaces:
      - faceIndex: 0
        faceId: <uuid>
        fromSegments: [d0000000-0000-0000-0000-000000000000]
      - faceIndex: 1
        faceId: <uuid>
        fromSegments: [e0000000-0000-0000-0000-000000000000]
  - id: body1
    surfaces:
      - faceIndex: 0
        faceId: <uuid>
        fromSegments: [a0000000-0000-0000-0000-000000000000, b0000000-0000-0000-0000-000000000000]
  - id: body2
    surfaces:
      - faceIndex: 0
        faceId: <uuid>
        fromSegments: [c0000000-0000-0000-0000-000000000000]

codecov · 2026-03-12T15:31:47Z

Codecov Report

❌ Patch coverage is 0% with 18 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@c7bef4f). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
modeling-cmds/src/shared.rs	0.00%	18 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1130   +/-   ##
=======================================
  Coverage        ?   30.01%           
=======================================
  Files           ?       34           
  Lines           ?     1609           
  Branches        ?        0           
=======================================
  Hits            ?      483           
  Misses          ?     1126           
  Partials        ?        0

Flag	Coverage Δ
unittests	`30.01% <0.00%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

davreev

Sounds good to me. If two consecutive segments are selected, would we still get two separate bodies or would they be merged into a single one?

adamchalmers · 2026-03-12T16:35:55Z

Sounds good to me. If two consecutive segments are selected, would we still get two separate bodies or would they be merged into a single one?

Good question. I'll ask @max-mrgrsk and @nicboone8. Perhaps it should be configurable.

nicboone8 · 2026-03-12T16:56:56Z

I would expect them merged by default, but making it configurable is a good idea

graphite-app · 2026-03-12T18:15:23Z

modeling-cmds/src/def_enum.rs

@@ -265,6 +302,15 @@ define_modeling_cmd_enum! {
            /// Which sketch to revolve.
            /// Must be a closed 2D solid.
            pub target: ModelingCmdId,


Documentation inconsistency: Revolve.target still says "Must be a closed 2D solid" but should be updated to "Must be a 2D sketch" to match the Extrude operation and reflect support for partial surface revolves via the new segments field.

Suggested change

/// Which sketch to sweep.

/// Must be a closed 2D solid.

pub target: ModelingCmdId,

/// If given, extrude each of these segments into a separate body with the given ID.

#[serde(default, skip_serializing_if = "Option::is_none")]

pub segments: Option<Vec<IdPair>>,

/// When two collinear segments are extruded, what should happen?

/// If true, creates a body with two surfaces.

/// If false, creates a body with one surface, spanning both collinear segments.

#[serde(default)]

#[builder(default)]

pub keep_seams: bool,

/// Path along which to sweep.

pub trajectory: ModelingCmdId,

/// If true, the sweep will be broken up into sub-sweeps (extrusions, revolves, sweeps) based on the trajectory path components.

/// Which sketch to revolve.

/// Must be a 2D sketch.

pub target: ModelingCmdId,

Spotted by Graphite

Is this helpful? React 👍 or 👎 to let us know.

graphite-app · 2026-03-25T17:22:54Z

modeling-cmds/openapi/api.json

              },
+              "segments": {
+                "nullable": true,
+                "description": "If given, extrude each of these segments into a separate body with the given ID.",


Misleading description: states "extrude each of these segments into a separate body with the given ID" but the array contains segment IDs to be extruded, not body IDs to assign. The wording "with the given ID" incorrectly suggests these UUIDs are for the resulting bodies. Should clarify:

"description": "If given, only extrude these segments from the sketch. Each segment will be extruded into a separate body."

This same incorrect description appears in all operation types (lines 2061, 2151, 2230, 2340, 2598).

Suggested change

"description": "If given, extrude each of these segments into a separate body with the given ID.",

"description": "If given, only extrude these segments from the sketch. Each segment will be extruded into a separate body.",

Spotted by Graphite

Is this helpful? React 👍 or 👎 to let us know.

This proposes a new way to surface extrude/revolve/sweep for sketch 2. Instead of extruding an entire sketch (solid2D) at once, we instead take a list of segments and extrude each of those separately. Each segment has a (segment ID, new body ID) pair, so the segment IDs are NOT absorbed into new body IDs. The sketch and all its segments are not modified.

Serena pointed out that if the extrude uses method=NEW, this will update old bodies by adding new surfaces, instead of creating new bodies. Let's be explicit about this.

graphite-app · 2026-03-27T18:04:25Z

modeling-cmds/src/shared.rs

+    /// All bodies created by this operation.
+    pub bodies: Vec<BodyUpdated>,


Incorrect documentation. The comment says "All bodies created by this operation" but this struct is BodiesUpdated, not BodiesCreated. The comment describes the wrong behavior.

Fix:

/// All bodies updated by this operation. pub bodies: Vec<BodyUpdated>,

Suggested change

/// All bodies created by this operation.

pub bodies: Vec<BodyUpdated>,

/// All bodies updated by this operation.

pub bodies: Vec<BodyUpdated>,

Spotted by Graphite

Is this helpful? React 👍 or 👎 to let us know.

graphite-app · 2026-03-30T22:24:57Z

modeling-cmds/src/ok_response.rs

+            #[serde(default, skip_serializing_if = "BodiesUpdated::is_empty")]
+            pub bodies_updated: BodiesUpdated,


Inconsistent field naming for bodies updated. The Extrude response uses bodies_updated but all other response structs (ExtrudeToReference, TwistExtrude, Sweep, Revolve, RevolveAboutEdge) use bodies for the same field type. This inconsistency will cause deserialization errors and confusion for API clients.

// Should be: pub bodies: BodiesUpdated,

Alternatively, all other structs should be changed to use bodies_updated for consistency.

Suggested change

#[serde(default, skip_serializing_if = "BodiesUpdated::is_empty")]

pub bodies_updated: BodiesUpdated,

#[serde(default, skip_serializing_if = "BodiesUpdated::is_empty")]

pub bodies: BodiesUpdated,

Spotted by Graphite

Is this helpful? React 👍 or 👎 to let us know.

benjamaan476 approved these changes Mar 12, 2026

View reviewed changes

cgoates approved these changes Mar 12, 2026

View reviewed changes

davreev approved these changes Mar 12, 2026

View reviewed changes

graphite-app bot reviewed Mar 12, 2026

View reviewed changes

pierremtb mentioned this pull request Mar 13, 2026

KCL: Segments from sketch blocks can be extruded KittyCAD/modeling-app#10413

Open

gserena01 approved these changes Mar 16, 2026

View reviewed changes

adamchalmers force-pushed the achalmers/extrude-segments branch from 485f01e to 06477d1 Compare March 18, 2026 19:12

graphite-app bot reviewed Mar 25, 2026

View reviewed changes

adamchalmers added 2 commits March 27, 2026 13:00

Allow updating existing bodies too

3004b11

Serena pointed out that if the extrude uses method=NEW, this will update old bodies by adding new surfaces, instead of creating new bodies. Let's be explicit about this.

adamchalmers force-pushed the achalmers/extrude-segments branch from cd56f6d to 3004b11 Compare March 27, 2026 18:00

graphite-app bot reviewed Mar 27, 2026

View reviewed changes

Merge branch 'main' into achalmers/extrude-segments

509f673

graphite-app bot reviewed Mar 30, 2026

View reviewed changes

gserena added 3 commits March 31, 2026 15:41

add from for the body types

fcd78f3

add for plurals

28b3371

Merge branch 'main' into achalmers/extrude-segments

60805c3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Non-destructive, partial surface extrudes/revolves/sweeps etc#1130

Non-destructive, partial surface extrudes/revolves/sweeps etc#1130
adamchalmers wants to merge 6 commits intomainfrom
achalmers/extrude-segments

adamchalmers commented Mar 12, 2026 •

edited

Loading

Uh oh!

codecov bot commented Mar 12, 2026 •

edited

Loading

Uh oh!

davreev left a comment •

edited

Loading

Uh oh!

adamchalmers commented Mar 12, 2026

Uh oh!

nicboone8 commented Mar 12, 2026

Uh oh!

graphite-app bot Mar 12, 2026

Uh oh!

graphite-app bot Mar 25, 2026

Uh oh!

graphite-app bot Mar 27, 2026

Uh oh!

graphite-app bot Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

+            /// Which sketch to sweep.
+            /// Must be a closed 2D solid.
+            pub target: ModelingCmdId,
+            /// If given, extrude each of these segments into a separate body with the given ID.
+            #[serde(default, skip_serializing_if = "Option::is_none")]
+            pub segments: Option<Vec<IdPair>>,
+            /// When two collinear segments are extruded, what should happen?
+            /// If true, creates a body with two surfaces.
+            /// If false, creates a body with one surface, spanning both collinear segments.
+            #[serde(default)]
+            #[builder(default)]
+            pub keep_seams: bool,
+            /// Path along which to sweep.
+            pub trajectory: ModelingCmdId,
+            /// If true, the sweep will be broken up into sub-sweeps (extrusions, revolves, sweeps) based on the trajectory path components.
+            /// Which sketch to revolve.
+            /// Must be a 2D sketch.
+            pub target: ModelingCmdId,

	"description": "If given, extrude each of these segments into a separate body with the given ID.",
	"description": "If given, only extrude these segments from the sketch. Each segment will be extruded into a separate body.",

		/// All bodies created by this operation.
		pub bodies: Vec<BodyUpdated>,

		#[serde(default, skip_serializing_if = "BodiesUpdated::is_empty")]
		pub bodies_updated: BodiesUpdated,

Conversation

adamchalmers commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Problem

Solution

Uh oh!

codecov bot commented Mar 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

davreev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamchalmers commented Mar 12, 2026

Uh oh!

nicboone8 commented Mar 12, 2026

Uh oh!

graphite-app bot Mar 12, 2026

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

graphite-app bot Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

adamchalmers commented Mar 12, 2026 •

edited

Loading

codecov bot commented Mar 12, 2026 •

edited

Loading

davreev left a comment •

edited

Loading