docs: add engine refactor design spec

Extract shared algorithms from DefaultNestEngine and StripNestEngine into focused helper classes: PairFiller, AngleCandidateBuilder, ShrinkFiller, RemnantFiller, AccumulatingProgress. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2026-03-16 21:44:59 -04:00
parent 5553fe6386
commit a510ac2acb
1 changed files with 191 additions and 0 deletions
--- a/docs/superpowers/specs/2026-03-16-engine-refactor-design.md
+++ b/docs/superpowers/specs/2026-03-16-engine-refactor-design.md
@@ -0,0 +1,191 @@
+# Engine Refactor: Extract Shared Algorithms from DefaultNestEngine and StripNestEngine
+
+## Problem
+
+`DefaultNestEngine` (~550 lines) mixes phase orchestration with strategy-specific logic (pair candidate selection, angle building, pattern helpers). `StripNestEngine` (~450 lines) duplicates patterns that DefaultNestEngine also uses: shrink-to-fit loops, iterative remnant filling, and progress accumulation. Both engines would benefit from extracting shared algorithms into focused, reusable classes.
+
+## Approach
+
+Extract five classes from the two engines. No new interfaces or strategy patterns — just focused helper classes that each engine composes.
+
+## Extracted Classes
+
+### 1. PairFiller
+
+**Source:** DefaultNestEngine lines 362-462 (`FillWithPairs`, `SelectPairCandidates`, `BuildRemainderPatterns`, `MinPairCandidates`, `PairTimeLimit`).
+
+**API:**
+```csharp
+public class PairFiller
+{
+    public PairFiller(Size plateSize, double partSpacing) { }
+
+    public List<Part> Fill(NestItem item, Box workArea,
+        CancellationToken token = default,
+        IProgress<NestProgress> progress = null);
+}
+```
+
+**Details:**
+- Constructor takes plate size and spacing — decoupled from `Plate` object.
+- `SelectPairCandidates` and `BuildRemainderPatterns` become private methods.
+- Uses `BestFitCache.GetOrCompute()` internally (same as today).
+- Calls `BuildRotatedPattern` and `FillPattern` — these become `internal static` methods on DefaultNestEngine so PairFiller can call them without ceremony.
+- Returns `List<Part>` (empty list if no result), same contract as today.
+
+**Caller:** `DefaultNestEngine.FindBestFill` replaces `this.FillWithPairs(...)` with `new PairFiller(Plate.Size, Plate.PartSpacing).Fill(...)`.
+
+### 2. AngleCandidateBuilder
+
+**Source:** DefaultNestEngine lines 279-347 (`BuildCandidateAngles`, `knownGoodAngles` HashSet, `ForceFullAngleSweep` property).
+
+**API:**
+```csharp
+public class AngleCandidateBuilder
+{
+    public bool ForceFullSweep { get; set; }
+
+    public List<double> Build(NestItem item, double bestRotation, Box workArea);
+
+    public void RecordProductive(List<AngleResult> angleResults);
+}
+```
+
+**Details:**
+- Owns `knownGoodAngles` state — lives as long as the engine instance so pruning accumulates across fills.
+- `Build()` encapsulates the full pipeline: base angles, sweep check, ML prediction, known-good pruning.
+- `RecordProductive()` replaces the inline loop that feeds `knownGoodAngles` after the linear phase.
+- `ForceFullAngleSweep` moves from DefaultNestEngine to here as `ForceFullSweep`.
+
+**Caller:** DefaultNestEngine creates one `AngleCandidateBuilder` instance in its constructor (or as a field) and calls `Build()`/`RecordProductive()` from `FindBestFill`.
+
+### 3. ShrinkFiller
+
+**Source:** StripNestEngine `TryOrientation` shrink loop (lines 188-215) and `ShrinkFill` (lines 358-418).
+
+**API:**
+```csharp
+public static class ShrinkFiller
+{
+    public static ShrinkResult Shrink(
+        Func<NestItem, Box, List<Part>> fillFunc,
+        NestItem item, Box box,
+        double spacing,
+        ShrinkAxis axis,
+        CancellationToken token = default,
+        int maxIterations = 20);
+}
+
+public enum ShrinkAxis { Width, Height }
+
+public class ShrinkResult
+{
+    public List<Part> Parts { get; set; }
+    public double Dimension { get; set; }
+}
+```
+
+**Details:**
+- `fillFunc` delegate decouples ShrinkFiller from any specific engine — the caller provides how to fill.
+- `ShrinkAxis` determines which dimension to reduce. `TryOrientation` passes the axis matching its strip direction. `ShrinkFill` calls `Shrink` twice (width then height).
+- Loop logic: fill initial box, measure placed bounding box, reduce dimension by `spacing`, retry until count drops below initial count.
+- Returns both the best parts and the final tight dimension (needed by `TryOrientation` to compute the remnant box).
+
+**Callers:**
+- `StripNestEngine.TryOrientation` replaces its inline shrink loop.
+- `StripNestEngine.ShrinkFill` replaces its two-axis inline shrink loops.
+
+### 4. RemnantFiller
+
+**Source:** StripNestEngine remnant loop (lines 262-342) and the simpler version in NestEngineBase.Nest (lines 74-97).
+
+**API:**
+```csharp
+public class RemnantFiller
+{
+    public RemnantFiller(Box workArea, double spacing) { }
+
+    public void AddObstacles(IEnumerable<Part> parts);
+
+    public List<Part> FillItems(
+        List<NestItem> items,
+        Func<NestItem, Box, List<Part>> fillFunc,
+        CancellationToken token = default,
+        IProgress<NestProgress> progress = null);
+}
+```
+
+**Details:**
+- Owns a `RemnantFinder` instance internally.
+- `AddObstacles` registers already-placed parts (bounding boxes offset by spacing).
+- `FillItems` runs the iterative loop: find remnants, try each item in each remnant, fill, update obstacles, repeat until no progress.
+- Local quantity tracking (dictionary keyed by drawing name) stays internal — does not mutate the input `NestItem` quantities. Returns the placed parts; the caller deducts quantities.
+- Uses minimum-remnant-size filtering (smallest remaining part dimension), same as StripNestEngine today.
+- `fillFunc` delegate allows callers to provide any fill strategy (DefaultNestEngine.Fill, ShrinkFill, etc.).
+
+**Callers:**
+- `StripNestEngine.TryOrientation` replaces its inline remnant loop with `RemnantFiller.FillItems(...)`.
+- `NestEngineBase.Nest` replaces its hand-rolled largest-remnant loop, gaining multi-remnant and minimum-size filtering for free.
+
+### 5. AccumulatingProgress
+
+**Source:** StripNestEngine nested class (lines 425-449).
+
+**API:**
+```csharp
+internal class AccumulatingProgress : IProgress<NestProgress>
+{
+    public AccumulatingProgress(IProgress<NestProgress> inner, List<Part> previousParts) { }
+    public void Report(NestProgress value);
+}
+```
+
+**Details:**
+- Moved from private nested class to standalone `internal` class in OpenNest.Engine.
+- No behavioral change — wraps an `IProgress<NestProgress>` and prepends previously placed parts to each report.
+
+## What Stays on Each Engine
+
+### DefaultNestEngine (~200 lines after extraction)
+
+- `Fill(NestItem, Box, ...)` — public entry point, unchanged.
+- `Fill(List<Part>, Box, ...)` — group-parts overload, unchanged.
+- `PackArea` — bin packing delegation, unchanged.
+- `FindBestFill` — orchestration, now ~30 lines: calls `AngleCandidateBuilder.Build()`, `PairFiller.Fill()`, linear angle loop, `FillRectangleBestFit`, picks best.
+- `FillRectangleBestFit` — 6-line private method, too small to extract.
+- `BuildRotatedPattern` / `FillPattern` — become `internal static`, used by both the linear loop and PairFiller.
+- `QuickFillCount` — stays (used by binary search, not shared).
+
+### StripNestEngine (~200 lines after extraction)
+
+- `Nest` — orchestration, unchanged.
+- `TryOrientation` — becomes thinner: calls `DefaultNestEngine.Fill` for initial fill, `ShrinkFiller.Shrink()` for tightening, `RemnantFiller.FillItems()` for remnants.
+- `ShrinkFill` — replaced by two `ShrinkFiller.Shrink()` calls.
+- `SelectStripItemIndex` / `EstimateStripDimension` — stay private, strip-specific.
+- `AccumulatingProgress` — removed, uses shared class.
+
+### NestEngineBase
+
+- `Nest` — switches from hand-rolled remnant loop to `RemnantFiller.FillItems()`.
+- All other methods unchanged.
+
+## File Layout
+
+All new classes go in `OpenNest.Engine/`:
+
+```
+OpenNest.Engine/
+    PairFiller.cs
+    AngleCandidateBuilder.cs
+    ShrinkFiller.cs
+    RemnantFiller.cs
+    AccumulatingProgress.cs
+```
+
+## Non-Goals
+
+- No new interfaces or strategy patterns.
+- No changes to FillLinear, FillBestFit, PackBottomLeft, or any other existing algorithm.
+- No changes to NestEngineRegistry or the plugin system.
+- No changes to public API surface — all existing callers continue to work unchanged.
+- PatternHelper extraction deferred — `BuildRotatedPattern`/`FillPattern` become `internal static` on DefaultNestEngine for now. Extract if a third consumer appears.