From d37b38622ff2c065cdf5139e008a1dff9dc315ab Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 5 Oct 2026 00:23:48 -0400 Subject: [PATCH] fix(cutting): treat classification, settings and malformed programs as stale Review of the atomic Apply found freshness gaps: - A drawing's cutoff classification decides lead, material, obstacle and dependency treatment but was not captured; changing it after planning still applied the old proposal. - Part and plate cutting settings were compared by reference only, so an in-place edit after capture applied (and a regenerated part overwrote it); plate settings were not compared at all. Settings are now captured as an exact public-state fingerprint. - A live program whose instruction list was set to null made Apply throw instead of returning Stale, and a respelled key in a case-insensitive binding dictionary compared equal. Caller-confirmed planning settings remain planning input: editing them after capture does not stale the plan and does not leak into the result. --- .../CNC/CuttingPlanning/PlateCuttingState.cs | 54 +++++++-- .../CNC/CuttingPlanning/ProgramContent.cs | 11 +- .../CNC/CuttingPlanning/StateFingerprint.cs | 107 ++++++++++++++++++ .../CuttingPlanning/CuttingDependencyTests.cs | 14 +++ .../CuttingPlanning/CuttingPlanCommitTests.cs | 53 ++++++++- 5 files changed, 226 insertions(+), 13 deletions(-) create mode 100644 OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs diff --git a/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs b/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs index 2cae3a8..a9694af 100644 --- a/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs +++ b/OpenNest.Core/CNC/CuttingPlanning/PlateCuttingState.cs @@ -2,6 +2,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading; +using OpenNest.CNC.CuttingStrategy; using OpenNest.Collections; using OpenNest.Geometry; @@ -9,8 +10,9 @@ namespace OpenNest.CNC.CuttingPlanning; /// /// Exact caller-thread record of everything a cutting proposal for one plate depends on: -/// part list instance and order, plate quantity/size/quadrant, cutoff definitions, and each -/// part's complete cutting state with owned copies of its placed and drawing programs. +/// part list instance and order, plate quantity/size/quadrant/settings, cutoff definitions, and +/// each part's complete cutting state with owned copies of its placed and drawing programs, +/// drawing cutoff classification and exact settings content. /// A commit compares it with the live plate and refuses when anything differs. /// public sealed class PlateCuttingState @@ -22,11 +24,15 @@ public sealed class PlateCuttingState private readonly int quadrant; private readonly ObservableList cutOffList; private readonly CutOffRecord[] cutOffs; - private readonly Dictionary drawings; + private readonly Dictionary drawings; + private readonly Dictionary settings; + private readonly CuttingParameters plateSettings; - private PlateCuttingState(Plate plate, PartRecord[] parts, - CutOffRecord[] cutOffs, Dictionary drawings) + private PlateCuttingState(Plate plate, PartRecord[] parts, CutOffRecord[] cutOffs, + Dictionary drawings, Dictionary settings) { + this.settings = settings; + plateSettings = plate.CuttingParameters; Plate = plate; partList = plate.Parts; this.parts = parts; @@ -53,7 +59,9 @@ public sealed class PlateCuttingState ArgumentNullException.ThrowIfNull(plate); if (plate.Parts == null || plate.CutOffs == null) throw new ArgumentException("Plate part and cutoff lists are required."); - var drawings = new Dictionary(ReferenceEqualityComparer.Instance); + var drawings = new Dictionary(ReferenceEqualityComparer.Instance); + var settings = new Dictionary(ReferenceEqualityComparer.Instance); + Fingerprint(plate.CuttingParameters); var records = new List(plate.Parts.Count); foreach (var part in plate.Parts) { @@ -63,14 +71,21 @@ public sealed class PlateCuttingState var drawing = part.BaseDrawing; if (!drawings.ContainsKey(drawing)) drawings.Add(drawing, (drawing.Program, drawing.Program == null ? null - : OwnedProgramCopy.Copy(drawing.Program, token))); + : OwnedProgramCopy.Copy(drawing.Program, token), drawing.IsCutOff)); var state = part.CaptureCuttingState(); + Fingerprint(state.CuttingParameters); records.Add(new(part, state, OwnedProgramCopy.Copy(part.Program, token), BoxValues(state.BoundingBox))); } var cutOffs = plate.CutOffs.Select(c => c == null ? throw new ArgumentException("Plate contains a missing cutoff definition.") : new CutOffRecord(c, c.Drawing, c.Axis, c.Position, c.StartLimit, c.EndLimit)).ToArray(); - return new(plate, records.ToArray(), cutOffs, drawings); + return new(plate, records.ToArray(), cutOffs, drawings, settings); + + void Fingerprint(CuttingParameters parameters) + { + if (parameters != null && !settings.ContainsKey(parameters)) + settings.Add(parameters, StateFingerprint.Of(parameters)); + } } /// True when the live plate still has exactly the captured state. @@ -85,6 +100,8 @@ public sealed class PlateCuttingState if (plate.Quantity != quantity || !Bits(plate.Size.Width, size.Width) || !Bits(plate.Size.Length, size.Length) || plate.Quadrant != quadrant) return "Plate quantity, size or quadrant changed."; + if (!ReferenceEquals(plate.CuttingParameters, plateSettings) || !SameSettings(plateSettings)) + return "Plate cutting settings changed."; if (plate.Parts.Count != parts.Length) return "Parts were added or removed."; for (var i = 0; i < parts.Length; i++) @@ -108,10 +125,14 @@ public sealed class PlateCuttingState return $"Part {i + 1} bounds changed."; if (!ProgramContent.Equal(part.Program, record.Program, token)) return $"Part {i + 1} program was edited in place."; - var (drawingProgram, drawingCopy) = drawings[part.BaseDrawing]; + if (!SameSettings(live.CuttingParameters)) + return $"Part {i + 1} cutting settings were edited in place."; + var (drawingProgram, drawingCopy, isCutOff) = drawings[part.BaseDrawing]; if (!ReferenceEquals(part.BaseDrawing.Program, drawingProgram) || !ProgramContent.Equal(drawingProgram, drawingCopy, token)) return $"Part {i + 1} drawing program changed."; + if (part.BaseDrawing.IsCutOff != isCutOff) + return $"Part {i + 1} cutoff classification changed."; } if (plate.CutOffs.Count != cutOffs.Length) return "Cutoffs were added or removed."; @@ -128,6 +149,21 @@ public sealed class PlateCuttingState // Part.Rotation derives from the manual flag, PreLeadInRotation and Program.Rotation, // all compared exactly above. return null; + + // References were compared already; this catches in-place edits of the same object. + bool SameSettings(CuttingParameters parameters) + { + if (parameters == null) + return true; + try + { + return settings.TryGetValue(parameters, out var captured) && captured == StateFingerprint.Of(parameters); + } + catch (NotSupportedException) + { + return false; // Grown past the exact fingerprint budget since capture. + } + } } private static long[] BoxValues(Box box) => box == null ? [] : diff --git a/OpenNest.Core/CNC/CuttingPlanning/ProgramContent.cs b/OpenNest.Core/CNC/CuttingPlanning/ProgramContent.cs index 72c6d2d..c21acdd 100644 --- a/OpenNest.Core/CNC/CuttingPlanning/ProgramContent.cs +++ b/OpenNest.Core/CNC/CuttingPlanning/ProgramContent.cs @@ -31,6 +31,9 @@ internal static class ProgramContent if (!programOwners.Add(b)) return false; programs.Add(a, b); + // Codes is a writable field: a missing list is a difference, never an exception. + if (a.Codes == null || b.Codes == null) + return a.Codes == null && b.Codes == null && a.GetType() == b.GetType(); if (a.GetType() != b.GetType() || a.Mode != b.Mode || !Bits(a.Rotation, b.Rotation) || a.Codes.Count != b.Codes.Count || a.SubPrograms.Count != b.SubPrograms.Count || !SameVariables(a.Variables, b.Variables)) @@ -81,7 +84,7 @@ internal static class ProgramContent { if (a == null || b == null) return a == null && b == null; - if (a.Count != b.Count || !a.Comparer.Equals(b.Comparer)) + if (a.Count != b.Count || !a.Comparer.Equals(b.Comparer) || !SameSpelling(a.Keys, b.Keys)) return false; foreach (var (key, value) in a) if (!b.TryGetValue(key, out var other) || !string.Equals(value, other, StringComparison.Ordinal)) @@ -89,9 +92,13 @@ internal static class ProgramContent return true; } + // A case-insensitive dictionary finds a respelled key; authored spelling must still match. + private static bool SameSpelling(IEnumerable a, IEnumerable b) => + new HashSet(a, StringComparer.Ordinal).SetEquals(b); + private static bool SameVariables(Dictionary a, Dictionary b) { - if (a.Count != b.Count || !a.Comparer.Equals(b.Comparer)) + if (a.Count != b.Count || !a.Comparer.Equals(b.Comparer) || !SameSpelling(a.Keys, b.Keys)) return false; foreach (var (key, value) in a) { diff --git a/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs b/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs new file mode 100644 index 0000000..7d0c8cd --- /dev/null +++ b/OpenNest.Core/CNC/CuttingPlanning/StateFingerprint.cs @@ -0,0 +1,107 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Globalization; +using System.Linq; +using System.Reflection; +using System.Text; + +namespace OpenNest.CNC.CuttingPlanning; + +/// +/// Exact text record of an object's public state (properties and fields, recursively; scalars by +/// bits, runtime types included) for detecting in-place edits of settings after capture. It is a +/// freshness fingerprint, not a serializer. Value types contribute fields only, so computed struct +/// properties cannot recurse without bound. +/// +internal static class StateFingerprint +{ + private const int MaxDepth = 16; + private const int NodeBudget = 100000; + + internal static string Of(object value) + { + var text = new StringBuilder(); + var path = new HashSet(ReferenceEqualityComparer.Instance); + var nodes = 0; + Append(value, 0); + return text.ToString(); + + void Append(object item, int depth) + { + if (++nodes > NodeBudget) + throw new NotSupportedException("Settings are too large to fingerprint exactly."); + if (item == null) + { + text.Append("null;"); + return; + } + var type = item.GetType(); + text.Append(type.FullName).Append('='); + switch (item) + { + case double number: + text.Append(BitConverter.DoubleToInt64Bits(number)).Append(';'); + return; + case float number: + text.Append(BitConverter.SingleToInt32Bits(number)).Append(';'); + return; + case string characters: + text.Append(characters.Length).Append(':').Append(characters).Append(';'); + return; + } + if (type.IsPrimitive || type.IsEnum || item is decimal) + { + text.Append(Convert.ToString(item, CultureInfo.InvariantCulture)).Append(';'); + return; + } + if (depth >= MaxDepth || (!type.IsValueType && !path.Add(item))) + { + text.Append(";"); + return; + } + try + { + text.Append('{'); + if (item is IEnumerable sequence) + { + foreach (var element in sequence) + Append(element, depth + 1); + } + else + { + foreach (var field in type.GetFields(BindingFlags.Public | BindingFlags.Instance) + .OrderBy(f => f.Name, StringComparer.Ordinal)) + { + text.Append(field.Name).Append(':'); + Append(field.GetValue(item), depth + 1); + } + if (!type.IsValueType) + foreach (var property in type.GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.CanRead && p.GetIndexParameters().Length == 0) + .OrderBy(p => p.Name, StringComparer.Ordinal)) + { + text.Append(property.Name).Append(':'); + object read; + try + { + read = property.GetValue(item); + } + catch (TargetInvocationException exception) + { + text.Append("throws ").Append(exception.InnerException?.GetType().FullName).Append(';'); + continue; + } + Append(read, depth + 1); + } + } + text.Append('}'); + } + finally + { + if (!type.IsValueType) + path.Remove(item); + } + } + } +} diff --git a/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs b/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs index 3432d5f..86dd311 100644 --- a/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs +++ b/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs @@ -39,6 +39,20 @@ public class CuttingDependencyTests Assert.Contains(q, plate.Parts); } + [Fact] + public void Apply_CutOffReclassifiedAfterPlanning_IsStale() + { + var (_, plate, p, q, cut) = CutOffPlate(ExplicitContourTests.Parameters(), orphan: false); + var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate, new Vector(11, 12.5))); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + cut.BaseDrawing.IsCutOff = false; + + var commit = CuttingPlanService.Apply([result]); + + Assert.Equal(CuttingCommitStatus.Stale, commit.Status); + Assert.Equal(new[] { p, q, cut }, plate.Parts); + } + [Fact] public void Plan_OrphanedCutOff_PrecedesEveryPart() { diff --git a/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs b/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs index 4abc7e7..4325318 100644 --- a/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs +++ b/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs @@ -45,16 +45,18 @@ public class CuttingPlanCommitTests [Fact] public void Apply_RegeneratedPlan_InstallsTheExactReplayedProgramWithOwnedSettings() { - var (nest, plate, part, parameters) = RegeneratedPlate(Vector.Zero); + var (nest, plate, part, _) = RegeneratedPlate(Vector.Zero); var location = part.Location; var rotation = part.Rotation; var quantity = part.BaseDrawing.Quantity.Nested; + // Caller-confirmed settings are planning input, not plate state: editing them after + // capture neither stales the plan nor leaks into what is installed. + var parameters = ExplicitContourTests.Parameters(); var length = ((LineLeadIn)parameters.ExternalLeadIn).Length; var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate, confirmedParameters: parameters)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); var proposal = Assert.Single(result.ProposedOrder); Assert.True(proposal.IsRegenerated); - // Settings edited after capture must not leak into what is installed. ((LineLeadIn)parameters.ExternalLeadIn).Length = length * 3; var commit = CuttingPlanService.Apply([result]); @@ -91,11 +93,17 @@ public class CuttingPlanCommitTests [InlineData("same-name-drawing")] [InlineData("list-replaced")] [InlineData("added")] + [InlineData("cutoff-classification")] + [InlineData("part-settings-in-place")] + [InlineData("part-settings-nested")] + [InlineData("plate-settings-in-place")] + [InlineData("plate-settings-replaced")] public void Apply_AnyChangeAfterCapture_IsStaleAndChangesNothing(string change) { var (_, plate, parts) = FixedPlate(); var cutOff = new CutOff(new Vector(30, 0), CutOffAxis.Vertical); plate.CutOffs.Add(cutOff); + plate.CuttingParameters = new CuttingParameters(); var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); switch (change) @@ -117,6 +125,13 @@ public class CuttingPlanCommitTests foreach (var part in parts) list.Add(part); plate.Parts = list; break; case "added": plate.Parts.Add(Rectangle(20, 0, 2, 2)); break; + // Classification decides lead, material, obstacle and dependency treatment. + case "cutoff-classification": parts[0].BaseDrawing.IsCutOff = true; break; + // A regenerated part would otherwise overwrite an edit made after capture. + case "part-settings-in-place": parts[0].CuttingParameters.Kerf = 0.125; break; + case "part-settings-nested": parts[0].CuttingParameters.Assignment.Preference = "LIAT"; break; + case "plate-settings-in-place": plate.CuttingParameters.PierceClearance = 0.25; break; + case "plate-settings-replaced": plate.CuttingParameters = new CuttingParameters(); break; } var after = PlateCuttingState.Capture(plate); var events = Watch(plate); @@ -130,6 +145,40 @@ public class CuttingPlanCommitTests Assert.Equal((0, 0, 0), events()); } + [Fact] + public void Apply_MalformedLiveProgram_IsStaleInsteadOfThrowing() + { + var (_, plate, parts) = FixedPlate(); + var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); + Assert.Equal(CuttingPlanStatus.Ready, result.Status); + parts[1].Program.Codes = null!; + var events = Watch(plate); + + var commit = CuttingPlanService.Apply([result]); + + Assert.Equal(CuttingCommitStatus.Stale, commit.Status); + Assert.Equal(parts, plate.Parts); + Assert.Null(parts[1].Program.Codes); + Assert.Equal((0, 0, 0), events()); + } + + [Fact] + public void ProgramContent_ComparesAuthoredKeySpellingExactly() + { + Program With(string key) + { + var program = new Program(); + program.Codes.Add(new LinearMove(1, 0) + { + VariableRefs = new Dictionary(StringComparer.OrdinalIgnoreCase) { [key] = "v" } + }); + return program; + } + + Assert.True(ProgramContent.Equal(With("X"), With("X"))); + Assert.False(ProgramContent.Equal(With("X"), With("x"))); + } + [Fact] public void Apply_LaterPlateStale_AppliesNothingAnywhere() {