Un-quarantine the internal-thread test: the geometry was right, the test was not

snaporca-kzy was filed as "internal thread cuts too little material". It does
not. Measured on the test's own fixture, a 40x40x20 box:

  plain Ø12 bore   removes 2261 mm3
  internal thread  removes 2157 = 1571 (minor bore) + 586 (groove)

apply_thread bores at the MINOR radius (radius - depth = 5) and then carves the
groove out to radius + depth = 7. A tapped hole therefore keeps the crests
between turns and holds MORE material than a plain clearance hole at the
nominal radius — which is what every real tapped hole does. The test asserted
the opposite, so it was asking for something physically wrong and had been
quarantined for it since it was written.

One hypothesis discarded on the way: that the shortfall was a tessellation
artefact, since chords on a helical surface undercut a concave bore. Exact
BRepGProp::VolumeProperties agreed with the tessellated volume to within
2.5 mm3, so that was not it and is not offered as a hedge.

The reference is now the tap-drill bore the thread actually starts from (Ø10),
against which the groove's 586 mm3 is the meaningful quantity — that is what
"the thread cuts" means. Test re-tagged [CadDocument][thread], so CI covers the
thread path again instead of skipping it.

Also documented the (void)internal in make_thread_profile. It reads like a bug
and is not: the V is the same shape either way and the caller decides, fusing
it onto a shaft or cutting it out of a wall. Someone "fixing" it to point
inward for the internal case would make the groove sweep already-empty bore
space and cut nothing — the exact failure the old comment described.

Suite 148 cases / 2035 assertions, with this test now among them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/libslic3r/CadDocument.cpp

@@ -151,6 +151,12 @@ static TopoDS_Wire make_thread_profile(const gp_Pnt& origin, const gp_Dir& xdir,
                                        const gp_Dir& zdir, double radius,
                                        double pitch, double depth, bool internal)
 {
+    // `internal` is intentionally unused: the V is the same shape either way, always pointing
+    // radially outward from `radius`. What differs is what the caller DOES with the swept solid
+    // — an external thread fuses it onto the shaft, an internal one cuts it out of the wall after
+    // boring at the minor diameter. Keeping the parameter documents that the caller decided, and
+    // stops someone "fixing" the profile to point inward for the internal case, which would make
+    // the groove sweep already-empty bore space and cut nothing.
     (void)internal;
     gp_Vec vx(xdir), vz(zdir);
     // Root the V CLEARLY inside the wall (a real overlap, not a 0.05 mm tangency) so the boolean

tests/libslic3r/test_caddocument.cpp

@@ -967,11 +967,17 @@ TEST_CASE("extrude taper + up-to-face distance", "[CadDocument]")
     }
 }
 
-// [known-broken]: pre-existing failure, the cut groove volume does not meet the asserted
-// threshold. Excluded from the delegated dev loop so a green run means "I broke nothing",
-// and [NotWorking] excludes it from CI's ctest gate for the same reason as the case above.
-// Tracked in the issue tracker instead — see snaporca-kzy.
-TEST_CASE("internal thread cuts a visible groove into the bore wall", "[CadDocument][known-broken][NotWorking]")
+// Was [known-broken] until the numbers were actually measured (snaporca-kzy). The geometry
+// was right all along; the TEST compared against the wrong reference. An internal thread bores
+// at the MINOR radius (radius - depth) and then carves the groove out to radius + depth, so a
+// tapped hole keeps the crests between turns and therefore holds MORE material than a plain
+// clearance hole at the nominal radius. Asserting the thread removes more than a plain Ø12 bore
+// asked for something no real tapped hole does. Measured on this fixture: plain Ø12 bore removes
+// 2261 mm3, the thread removes 2157 = 1571 (minor bore) + 586 (groove). Exact BRepGProp volume
+// agrees with the tessellated volume to 2.5 mm3, so this was never a meshing artefact either.
+// The tap-drill bore is the honest reference: against it, the groove's 586 mm3 is the thing worth
+// asserting, because that is what "the thread actually cuts" means.
+TEST_CASE("internal thread cuts a visible groove into the bore wall", "[CadDocument][thread]")
 {
     using namespace Slic3r;
     SketchPlane xy = SketchPlane::XY();
@@ -992,10 +998,11 @@ TEST_CASE("internal thread cuts a visible groove into the bore wall", "[CadDocum
         doc.features.push_back(ex);
     };
 
-    // Reference: box with a plain Ø12 bore through it.
+    // Reference: box bored at the MINOR diameter — the tap drill the thread starts from.
+    // Ø10 = 2 * (radius 6 - depth 1), matching what apply_thread cuts before the groove.
     CadDocument hole_doc;
     make_box(hole_doc);
-    hole_doc.add_hole(12.0, 20.0, true, 0.0, 0.0, xy, "Hole");
+    hole_doc.add_hole(10.0, 20.0, true, 0.0, 0.0, xy, "Hole");
     REQUIRE(hole_doc.recompute());
     REQUIRE(hole_doc.error.empty());
     const double v_hole = double(hole_doc.display_mesh.volume());
@@ -1008,10 +1015,9 @@ TEST_CASE("internal thread cuts a visible groove into the bore wall", "[CadDocum
     REQUIRE(thr_doc.error.empty());
     const double v_thread = double(thr_doc.display_mesh.volume());
 
-    // The helical groove must carve material OUT of the wall, beyond the plain
-    // bore -> a visible internal thread. The old inward-pointing profile only
-    // swept already-empty bore space and removed essentially nothing, so it would
-    // give v_thread ~= v_hole; the fixed profile removes a meaningful volume.
+    // The groove must carve material out of the wall BEYOND the tap-drill bore, which is what
+    // makes the thread visible. A profile that only swept already-empty bore space would give
+    // v_thread ~= v_hole; the real one removes several hundred mm3 more.
     REQUIRE(v_thread > 0.0);
     REQUIRE(v_thread < v_hole);
     REQUIRE((v_hole - v_thread) > 20.0);