From 360f58c101f6d29e1a7627e9c6bfa4c5bb35d28f Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Tue, 13 Oct 2026 17:22:50 -0400 Subject: [PATCH] fix(engine): record a committed clamp removal whatever the target sheet holds SyncClampZones returned early when the empty target already held exactly the solved reduced zones, and otherwise preferred the target's own rail over the stock's, so a removal committed onto such a sheet kept no rail, removed ids or time. The stock's rail now wins, and the rail/removal state is reconciled even when the zones themselves need no replacement. PM 60f7a673-455e-4a87-b865-a4806f417ee6 --- .../Jobs/ClampRemovalRepairTests.cs | 28 +++++++++++++++++++ OpenNest.Engine/Jobs/NestPipelineCommit.cs | 24 ++++++++-------- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/OpenNest.Engine.Tests/Jobs/ClampRemovalRepairTests.cs b/OpenNest.Engine.Tests/Jobs/ClampRemovalRepairTests.cs index 3a1780e..8e8c78a 100644 --- a/OpenNest.Engine.Tests/Jobs/ClampRemovalRepairTests.cs +++ b/OpenNest.Engine.Tests/Jobs/ClampRemovalRepairTests.cs @@ -279,4 +279,32 @@ public class ClampRemovalRepairTests Assert.Null(outcome); Assert.True(engine.Calls <= ClampRemovalRecovery.MaxProbeSolves, $"recovery made {engine.Calls} solves"); } + // PM 60f7a673: the removal is recorded whatever the empty target sheet already holds. + [Theory] + [InlineData("reduced-explicit")] // no rail, already exactly the solved reduced zones + [InlineData("other-rail")] // follows a different rail that cannot recreate them + public void CommittedRemovalIsRecordedOnAnyEmptyTarget(string target) + { + var drawing = new Drawing("wide", TestDrawingFactory.Rectangle(14, 3)); + var nest = new Nest(); + nest.Drawings.Add(drawing); + nest.PlateDefaults.Size = new Size(3, 23); + nest.PlateDefaults.EdgeSpacing = new Spacing(0, 0, 0, 0); + var plate = nest.CreatePlate(); + if (target == "reduced-explicit") + plate.ClampZones.Add(Zones()[1]); + else + plate.SetClampRail(new ClampRail(2, 0.625, new[] { 6.0, 21.0 })); + using var manager = new PlateManager(nest); + + var result = NestPipeline.Run(Request(Stock(), Item("wide", 14, 3, 1))); + Assert.True(result.IsValid, string.Join("; ", result.Violations)); + var applied = Assert.Single(NestPipelineCommit.ApplyToEmptyPlates(result, manager)); + + Assert.Same(plate, applied); + Assert.Same(Rail, applied.ClampRail); + Assert.Equal(new[] { "clamp-1" }, applied.RemovedClamps); + Assert.Equal(new[] { "clamp-2" }, applied.ClampZones.Select(z => z.Id)); + Assert.Equal(ClampRail.DefaultRemovalMinutes, applied.ClampRemovalMinutes); + } } diff --git a/OpenNest.Engine/Jobs/NestPipelineCommit.cs b/OpenNest.Engine/Jobs/NestPipelineCommit.cs index d00d6b8..86a282b 100644 --- a/OpenNest.Engine/Jobs/NestPipelineCommit.cs +++ b/OpenNest.Engine/Jobs/NestPipelineCommit.cs @@ -68,27 +68,29 @@ public static class NestPipelineCommit { var left = stock.Quadrant is 1 or 4 ? 0 : -stock.Size.Length; var bottom = stock.Quadrant is 1 or 2 ? 0 : -stock.Size.Width; - if (SameInPlacementFrame(plate.ClampZones, stock.ClampZones, left, bottom)) - return; - - // A plate without a rail still takes the rail its stock was resolved from, so a - // last-resort removal on that stock is never committed without naming the clamp. - var rail = plate.ClampRail ?? stock.ClampRail; - plate.SetClampRail(null); // explicit zones from here on - foreach (var zone in stock.ClampZones) - plate.ClampZones.Add(new ClampZone(zone.Id, zone.Bounds.Translate(-left, -bottom))); + // The rail the solved zones were resolved from wins over whatever the target follows; + // explicit stock keeps the target's own rail as before. + var rail = stock.ClampRail ?? plate.ClampRail; + if (!SameInPlacementFrame(plate.ClampZones, stock.ClampZones, left, bottom)) + { + plate.SetClampRail(null); // explicit zones from here on + foreach (var zone in stock.ClampZones) + plate.ClampZones.Add(new ClampZone(zone.Id, zone.Bounds.Translate(-left, -bottom))); + } if (rail == null) return; // D8: the sheet's zones may be the rail's resolution for it minus clamps Auto Nest // removed as a last resort; derive which (if any) so the committed plate keeps - // following the rail - and the removal - through later resizes (C13). + // following the rail - and the removal - through later resizes (C13). This runs even + // when the target already held exactly these zones, so the removal is never lost. var kept = new HashSet(plate.ClampZones.Select(z => z.Id), StringComparer.Ordinal); var removedClamps = rail.ResolveFor(plate.Size, plate.Quadrant) .Select(z => z.Id) .Where(id => !kept.Contains(id)) .ToList(); - if (rail.Matches(plate, removedClamps)) + var unchanged = ReferenceEquals(plate.ClampRail, rail) && plate.RemovedClamps.SetEquals(removedClamps); + if (!unchanged && rail.Matches(plate, removedClamps)) plate.SetClampRail(rail, removedClamps); }