From f901840ed073d4c60ae35f1a40fb89208c034eba Mon Sep 17 00:00:00 2001
From: AJ Isaacs <ajisaacs27@gmail.com>
Date: Tue, 20 Oct 2026 21:04:47 -0400
Subject: [PATCH] fix(flatbar): stop the swap optimization trading identical
 copies

TryImprove kept a swap when the trial run looked fuller than before, but
the trial charges each type's self-nesting gain again even when the run
already holds that type. Trading a placed copy for an identical unplaced
one therefore looked fuller by the gain and repeated forever: FlatBar ran
into the benchmark's 5-minute timeout on tabbed_cog_layout once the
all-edges MBR fix gave one part new rotations (it finished in 7 s before).

The swap is now judged on the merged run, so every kept swap really
leaves the run fuller and the loop terminates. Swaps the old test kept
that do fill the run are still kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---
 .../NestingEngines/BarPackerTests.cs          | 31 +++++++++++++++++++
 .../NestingEngines/FlatBar/BarPacker.cs       |  8 ++++-
 2 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 OpenNest.Engine.Tests/NestingEngines/BarPackerTests.cs

diff --git a/OpenNest.Engine.Tests/NestingEngines/BarPackerTests.cs b/OpenNest.Engine.Tests/NestingEngines/BarPackerTests.cs
new file mode 100644
index 00000000..25aba0b4
--- /dev/null
+++ b/OpenNest.Engine.Tests/NestingEngines/BarPackerTests.cs
@@ -0,0 +1,31 @@
+using System.Linq;
+using System.Threading;
+using OpenNest.Engine.NestingEngines.FlatBar;
+
+namespace OpenNest.Engine.Tests.NestingEngines;
+
+public class BarPackerTests
+{
+    [Fact]
+    public void SwapOptimizationStopsWhenOnlyIdenticalCopiesRemain()
+    {
+        // One bar holds [6, 1.5, 1.5] with 0.99 free; two 1.5 copies are left over. Trading a
+        // placed copy for an unplaced one changes nothing, but a trial charges the copy's
+        // self-nesting gain again and looked 0.01 fuller, so the swap used to repeat forever.
+        var items = new[]
+        {
+            new BarItem(0, 6, 0, 0),
+            new BarItem(1, 1.5, 0, 0.01),
+            new BarItem(1, 1.5, 0, 0.01),
+            new BarItem(1, 1.5, 0, 0.01),
+            new BarItem(1, 1.5, 0, 0.01),
+        };
+        using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(10));
+
+        var plan = BarPacker.Pack(items, [10.0], 1, timeout.Token);
+
+        var bar = Assert.Single(plan.Bars);
+        Assert.Equal(new[] { 0, 1, 1 }, bar.Items.Select(i => i.Type).OrderBy(t => t));
+        Assert.Equal(2, plan.Unplaced.Count);
+    }
+}
diff --git a/OpenNest.Engine/NestingEngines/FlatBar/BarPacker.cs b/OpenNest.Engine/NestingEngines/FlatBar/BarPacker.cs
index e9132956..96dd12e2 100644
--- a/OpenNest.Engine/NestingEngines/FlatBar/BarPacker.cs
+++ b/OpenNest.Engine/NestingEngines/FlatBar/BarPacker.cs
@@ -174,7 +174,13 @@ internal static class BarPacker
                     trial.Items.Add(next);
                 }
 
-                if (trial.Remaining < before)
+                // Judge the swap on the merged run: the trial charges gains again for types the
+                // run already holds, so trading a copy for an identical one looks fuller and the
+                // swap would repeat forever.
+                var merged = new PackedSegment(segment.Capacity);
+                merged.Items.AddRange(segment.Items);
+                merged.Items.AddRange(trial.Items);
+                if (merged.Remaining < before)
                 {
                     remaining.Add(removed);
                     segment.Items.AddRange(trial.Items);