Files
OpenNest/docs/superpowers/plans/2026-03-18-refactor-compactor.md
2026-03-18 20:12:23 -04:00

14 KiB

Refactor Compactor Implementation Plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: Prune dead code from Compactor and deduplicate the Push overloads into a single scanning core.

Architecture: Delete 6 unused methods (Compact, CompactLoop, SavePositions, RestorePositions, CompactIndividual, CompactIndividualLoop). Unify the Push(... PushDirection) core overload to convert its PushDirection to a unit Vector and delegate to the Push(... Vector) overload, eliminating ~60 lines of duplicated obstacle scanning logic. PushBoundingBox stays separate since it's a fundamentally different algorithm (no geometry lines).

Tech Stack: C# / .NET 8


Task 1: Write Compactor Push tests as a safety net

No Compactor tests exist. Before changing anything, add tests for the public Push methods that have live callers: Push(parts, obstacles, workArea, spacing, PushDirection) and Push(parts, obstacles, workArea, spacing, angle).

Files:

  • Create: OpenNest.Tests/CompactorTests.cs

  • Step 1: Write tests for Push with PushDirection

using OpenNest;
using OpenNest.Engine.Fill;
using OpenNest.Geometry;
using Xunit;
using System.Collections.Generic;

namespace OpenNest.Tests
{
    public class CompactorTests
    {
        private static Drawing MakeRectDrawing(double w, double h)
        {
            var pgm = new OpenNest.CNC.Program();
            pgm.Codes.Add(new OpenNest.CNC.RapidMove(new Vector(0, 0)));
            pgm.Codes.Add(new OpenNest.CNC.LinearMove(new Vector(w, 0)));
            pgm.Codes.Add(new OpenNest.CNC.LinearMove(new Vector(w, h)));
            pgm.Codes.Add(new OpenNest.CNC.LinearMove(new Vector(0, h)));
            pgm.Codes.Add(new OpenNest.CNC.LinearMove(new Vector(0, 0)));
            return new Drawing("rect", pgm);
        }

        private static Part MakeRectPart(double x, double y, double w, double h)
        {
            var drawing = MakeRectDrawing(w, h);
            var part = new Part(drawing) { Location = new Vector(x, y) };
            part.UpdateBounds();
            return part;
        }

        [Fact]
        public void Push_Left_MovesPartTowardEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            var distance = Compactor.Push(moving, obstacles, workArea, 0, PushDirection.Left);

            Assert.True(distance > 0);
            Assert.True(part.BoundingBox.Left < 1);
        }

        [Fact]
        public void Push_Left_StopsAtObstacle()
        {
            var workArea = new Box(0, 0, 100, 100);
            var obstacle = MakeRectPart(0, 0, 10, 10);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part> { obstacle };

            Compactor.Push(moving, obstacles, workArea, 0, PushDirection.Left);

            Assert.True(part.BoundingBox.Left >= obstacle.BoundingBox.Right - 0.1);
        }

        [Fact]
        public void Push_Down_MovesPartTowardEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(0, 50, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            var distance = Compactor.Push(moving, obstacles, workArea, 0, PushDirection.Down);

            Assert.True(distance > 0);
            Assert.True(part.BoundingBox.Bottom < 1);
        }

        [Fact]
        public void Push_ReturnsZero_WhenAlreadyAtEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(0, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            var distance = Compactor.Push(moving, obstacles, workArea, 0, PushDirection.Left);

            Assert.Equal(0, distance);
        }

        [Fact]
        public void Push_WithSpacing_MaintainsGap()
        {
            var workArea = new Box(0, 0, 100, 100);
            var obstacle = MakeRectPart(0, 0, 10, 10);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part> { obstacle };

            Compactor.Push(moving, obstacles, workArea, 2, PushDirection.Left);

            Assert.True(part.BoundingBox.Left >= obstacle.BoundingBox.Right + 2 - 0.5);
        }
    }
}
  • Step 2: Write tests for Push with angle (Vector-based)

Add to the same file:

        [Fact]
        public void Push_AngleLeft_MovesPartTowardEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            // angle = π = push left
            var distance = Compactor.Push(moving, obstacles, workArea, 0, System.Math.PI);

            Assert.True(distance > 0);
            Assert.True(part.BoundingBox.Left < 1);
        }

        [Fact]
        public void Push_AngleDown_MovesPartTowardEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(0, 50, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            // angle = 3π/2 = push down
            var distance = Compactor.Push(moving, obstacles, workArea, 0, 3 * System.Math.PI / 2);

            Assert.True(distance > 0);
            Assert.True(part.BoundingBox.Bottom < 1);
        }
  • Step 3: Write tests for PushBoundingBox

Add to the same file:

        [Fact]
        public void PushBoundingBox_Left_MovesPartTowardEdge()
        {
            var workArea = new Box(0, 0, 100, 100);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part>();

            var distance = Compactor.PushBoundingBox(moving, obstacles, workArea, 0, PushDirection.Left);

            Assert.True(distance > 0);
            Assert.True(part.BoundingBox.Left < 1);
        }

        [Fact]
        public void PushBoundingBox_StopsAtObstacle()
        {
            var workArea = new Box(0, 0, 100, 100);
            var obstacle = MakeRectPart(0, 0, 10, 10);
            var part = MakeRectPart(50, 0, 10, 10);
            var moving = new List<Part> { part };
            var obstacles = new List<Part> { obstacle };

            Compactor.PushBoundingBox(moving, obstacles, workArea, 0, PushDirection.Left);

            Assert.True(part.BoundingBox.Left >= obstacle.BoundingBox.Right - 0.1);
        }
  • Step 4: Run tests to verify they pass

Run: dotnet test OpenNest.Tests --filter "FullyQualifiedName~CompactorTests" -v n Expected: All tests PASS (these test existing behavior before refactoring)

  • Step 5: Commit
git add OpenNest.Tests/CompactorTests.cs
git commit -m "test: add Compactor safety-net tests before refactor"

Task 2: Delete dead code

Remove the 6 methods that have zero live callers: Compact, CompactLoop, SavePositions, RestorePositions, CompactIndividual, CompactIndividualLoop. Also remove the unused contactGap variable (line 181) and the misplaced XML doc comment above RepeatThreshold (lines 16-20, describes the deleted Compact method).

Files:

  • Modify: OpenNest.Engine/Fill/Compactor.cs

  • Step 1: Delete SavePositions and RestorePositions

Delete SavePositions (lines 64-70) and RestorePositions (lines 72-76). These are only used by Compact and CompactIndividual.

  • Step 2: Delete Compact and CompactLoop

Delete the Compact method (lines 24-44) and CompactLoop method (lines 46-62). Zero callers.

  • Step 3: Delete CompactIndividual and CompactIndividualLoop

Delete CompactIndividual (lines 312-332) and CompactIndividualLoop (lines 334-360). Only caller is a commented-out line in StripNestEngine.cs:189.

  • Step 4: Remove the commented-out caller in StripNestEngine

In OpenNest.Engine/StripNestEngine.cs, delete the entire commented-out block (lines 186-194):

            // TODO: Compact strip parts individually to close geometry-based gaps.
            // Disabled pending investigation — remnant finder picks up gaps created
            // by compaction and scatters parts into them.
            // Compactor.CompactIndividual(bestParts, workArea, Plate.PartSpacing);
            //
            // var compactedBox = bestParts.Cast<IBoundable>().GetBoundingBox();
            // bestDim = direction == StripDirection.Bottom
            //     ? compactedBox.Top - workArea.Y
            //     : compactedBox.Right - workArea.X;
  • Step 5: Clean up stale doc comment and dead variable

Remove the orphaned XML doc comment above RepeatThreshold (lines 16-20 — it describes the deleted Compact method). Remove the RepeatThreshold and MaxIterations constants (only used by the deleted loop methods). Remove the unused contactGap variable from the Push(... PushDirection) method (line 181).

  • Step 6: Run tests

Run: dotnet test OpenNest.Tests --filter "FullyQualifiedName~CompactorTests" -v n Expected: All tests PASS (deleted code was unused)

  • Step 7: Build full solution to verify no compilation errors

Run: dotnet build OpenNest.sln Expected: Build succeeded, 0 errors

  • Step 8: Commit
git add OpenNest.Engine/Fill/Compactor.cs OpenNest.Engine/StripNestEngine.cs
git commit -m "refactor(compactor): remove dead code — Compact, CompactIndividual, and helpers"

Task 3: Deduplicate Push overloads

The Push(... PushDirection) core overload (lines 166-238) duplicates the obstacle scanning loop from Push(... Vector) (lines 102-164). Convert PushDirection to a unit Vector and delegate.

Files:

  • Modify: OpenNest.Engine/Fill/Compactor.cs

  • Step 1: Replace the Push(... PushDirection) core overload

Replace the full body of Push(List<Part> movingParts, List<Part> obstacleParts, Box workArea, double partSpacing, PushDirection direction) with a delegation to the Vector overload:

public static double Push(List<Part> movingParts, List<Part> obstacleParts,
    Box workArea, double partSpacing, PushDirection direction)
{
    var vector = SpatialQuery.DirectionToOffset(direction, 1.0);
    return Push(movingParts, obstacleParts, workArea, partSpacing, vector);
}

This works because DirectionToOffset(Left, 1.0) returns (-1, 0), which is the unit vector for "push left" — exactly what new Vector(Math.Cos(π), Math.Sin(π)) produces. The Vector overload already handles edge distance, obstacle scanning, geometry lines, and offset application identically.

  • Step 2: Update the angle-based Push to accept Vector directly

Rename the existing Push(... double angle) core overload to accept a Vector direction instead of computing it internally. This avoids a redundant cos/sin when the PushDirection overload already provides a unit vector.

Change the signature from:

public static double Push(List<Part> movingParts, List<Part> obstacleParts,
    Box workArea, double partSpacing, double angle)

to:

public static double Push(List<Part> movingParts, List<Part> obstacleParts,
    Box workArea, double partSpacing, Vector direction)

Remove the var direction = new Vector(...) line from the body since direction is now a parameter.

  • Step 3: Update the angle convenience overload to convert

The convenience overload Push(List<Part> movingParts, Plate plate, double angle) must now convert the angle to a Vector before calling the core:

public static double Push(List<Part> movingParts, Plate plate, double angle)
{
    var obstacleParts = plate.Parts
        .Where(p => !movingParts.Contains(p))
        .ToList();

    var direction = new Vector(System.Math.Cos(angle), System.Math.Sin(angle));
    return Push(movingParts, obstacleParts, plate.WorkArea(), plate.PartSpacing, direction);
}
  • Step 4: Run tests

Run: dotnet test OpenNest.Tests --filter "FullyQualifiedName~CompactorTests" -v n Expected: All tests PASS

  • Step 5: Build full solution

Run: dotnet build OpenNest.sln Expected: Build succeeded, 0 errors. All callers in FillExtents, ActionClone, PlateView, PatternTileForm compile without changes — their call signatures are unchanged.

  • Step 6: Commit
git add OpenNest.Engine/Fill/Compactor.cs
git commit -m "refactor(compactor): deduplicate Push — PushDirection delegates to Vector overload"

Task 4: Final cleanup and verify

Files:

  • Modify: OpenNest.Engine/Fill/Compactor.cs (if needed)

  • Step 1: Run full test suite

Run: dotnet test OpenNest.Tests -v n Expected: All tests PASS

  • Step 2: Verify Compactor is clean

The final Compactor should have 6 public methods:

  1. Push(parts, plate, PushDirection) — convenience, extracts plate fields
  2. Push(parts, plate, angle) — convenience, converts angle to Vector
  3. Push(parts, obstacles, workArea, spacing, PushDirection) — converts to Vector, delegates
  4. Push(parts, obstacles, workArea, spacing, Vector) — the single scanning core
  5. PushBoundingBox(parts, plate, direction) — convenience
  6. PushBoundingBox(parts, obstacles, workArea, spacing, direction) — BB-only core

Plus one constant: ChordTolerance.

File should be ~110-120 lines, down from 362.