From 1d8c5fe2741447c326274fa942ce4e3ebf2f17de Mon Sep 17 00:00:00 2001 From: AJ Isaacs Date: Mon, 5 Oct 2026 00:28:59 -0400 Subject: [PATCH] fix(cutting): create plate-scoped requests through a factory A CuttingPlanRequest constructor overload taking a Plate made the existing detached call new CuttingPlanRequest(null) ambiguous (CS0121). Plate scope is now requested with CuttingPlanRequest.ForPlate, and the result summary describes dependencies and Apply as they now behave. --- .../CuttingPlanning/CuttingPlanModels.cs | 23 +++++----- .../CuttingPlanning/CuttingDependencyTests.cs | 20 ++++----- .../CuttingPlanning/CuttingPlanCommitTests.cs | 42 +++++++++++-------- docs/cutting-planner.md | 6 +-- 4 files changed, 50 insertions(+), 41 deletions(-) diff --git a/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs b/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs index e3f3d98..1e165dc 100644 --- a/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs +++ b/OpenNest.Engine/CuttingPlanning/CuttingPlanModels.cs @@ -11,7 +11,8 @@ namespace OpenNest.Engine.CuttingPlanning; /// /// Caller-side direct-XY input. Regeneration requires explicit confirmed parameters. -/// Keep sources/settings stable during Capture. This is not an Apply request. +/// Keep sources/settings stable during Capture. Only plate-scoped requests () +/// record the state that CuttingPlanService.Apply checks; a detached part list cannot be applied. /// public sealed class CuttingPlanRequest { @@ -30,19 +31,18 @@ public sealed class CuttingPlanRequest /// /// Plate scope: plans the plate's current parts and captures its exact state, so a Ready - /// result can later be applied through . + /// result can later be applied through CuttingPlanService.Apply. A factory rather than a + /// constructor overload, so new CuttingPlanRequest(null) stays unambiguous. /// - public CuttingPlanRequest(Plate plate, Vector startPoint = default, int expansionBudget = 20000, + public static CuttingPlanRequest ForPlate(Plate plate, Vector startPoint = default, int expansionBudget = 20000, CuttingParameters confirmedParameters = null, IEnumerable eligibleParts = null, - bool preservePartOrder = false, int maxEntries = 16) - : this(plate?.Parts, startPoint, expansionBudget, confirmedParameters, eligibleParts, + bool preservePartOrder = false, int maxEntries = 16) => + new(plate?.Parts, startPoint, expansionBudget, confirmedParameters, eligibleParts, preservePartOrder, maxEntries) - { - Plate = plate; - } + { Plate = plate }; /// The plate scope, or null for a detached part list that cannot be applied. - public Plate Plate { get; } + public Plate Plate { get; private init; } public CuttingParameters ConfirmedParameters { get; } public IReadOnlyList EligibleParts { get; } public bool PreservePartOrder { get; } @@ -167,8 +167,9 @@ public sealed record CuttingPlanFinding(int? SourceOrdinal, Part SourcePart, int? OtherSourceOrdinal, Part OtherSourcePart, PostVerificationKind? Kind, string Message); /// -/// A replayed direct-XY proposal, optionally with regenerated programs. Not physical -/// safety, posting consent, dependency readiness or an atomic Apply payload. Failures contain no proposals. +/// A replayed direct-XY proposal, optionally with regenerated programs, that respects the +/// captured cutoff/containment prerequisites. Not physical safety or posting consent. Apply +/// installs it only for a plate-scoped request whose plate is unchanged. Failures contain no proposals. /// public sealed class CuttingPlanResult { diff --git a/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs b/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs index eb89727..64270a7 100644 --- a/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs +++ b/OpenNest.Tests/CuttingPlanning/CuttingDependencyTests.cs @@ -18,7 +18,7 @@ public class CuttingDependencyTests { var parameters = ExplicitContourTests.Parameters(); var (_, plate, p, q, cut) = CutOffPlate(parameters, orphan: false); - var request = new CuttingPlanRequest(plate, new Vector(11, 12.5), + var request = CuttingPlanRequest.ForPlate(plate, new Vector(11, 12.5), confirmedParameters: regenerate ? parameters : null); var snapshot = CuttingPlanService.Capture(request); Assert.Equal(new[] { 2 }, snapshot.Dependencies.PrerequisitesOf(0)); @@ -43,7 +43,7 @@ public class CuttingDependencyTests 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))); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, new Vector(11, 12.5))); Assert.Equal(CuttingPlanStatus.Ready, result.Status); cut.BaseDrawing.IsCutOff = false; @@ -57,7 +57,7 @@ public class CuttingDependencyTests public void Plan_OrphanedCutOff_PrecedesEveryPart() { var (_, plate, p, q, cut) = CutOffPlate(ExplicitContourTests.Parameters(), orphan: true); - var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest(plate, new Vector(11, 12.5))); + var snapshot = CuttingPlanService.Capture(CuttingPlanRequest.ForPlate(plate, new Vector(11, 12.5))); Assert.Equal(new[] { 2 }, snapshot.Dependencies.PrerequisitesOf(0)); Assert.Equal(new[] { 2 }, snapshot.Dependencies.PrerequisitesOf(1)); @@ -72,7 +72,7 @@ public class CuttingDependencyTests { var (_, plate, p, _, cut) = CutOffPlate(ExplicitContourTests.Parameters(), orphan: false); - var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate, new Vector(11, 12.5), preservePartOrder: true)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, new Vector(11, 12.5), preservePartOrder: true)); Assert.Equal(CuttingPlanStatus.ConstraintConflict, result.Status); Assert.Empty(result.ProposedOrder); @@ -89,7 +89,7 @@ public class CuttingDependencyTests { var parameters = ExplicitContourTests.Parameters(); var (_, plate, _, _, _) = CutOffPlate(parameters, orphan: false); - var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest(plate, new Vector(11, 12.5), + var snapshot = CuttingPlanService.Capture(CuttingPlanRequest.ForPlate(plate, new Vector(11, 12.5), confirmedParameters: regenerate ? parameters : null)); var ready = CuttingPlanService.Plan(snapshot); Assert.Equal(CuttingPlanStatus.Ready, ready.Status); @@ -120,7 +120,7 @@ public class CuttingDependencyTests public void Plan_PartInsideAHostCutout_IsCutBeforeTheHost() { var (plate, host, inner) = NestedPlate(Rectangle(2.7, 2.7, 0.6, 0.6)); - var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest(plate)); + var snapshot = CuttingPlanService.Capture(CuttingPlanRequest.ForPlate(plate)); Assert.Equal(new[] { 1 }, snapshot.Dependencies.PrerequisitesOf(0)); Assert.Empty(snapshot.Dependencies.PrerequisitesOf(1)); @@ -129,7 +129,7 @@ public class CuttingDependencyTests Assert.True(result.Status == CuttingPlanStatus.Ready, Describe(result)); Assert.Equal(new[] { inner, host }, result.ProposedOrder.Select(o => o.SourcePart)); - var preserved = CuttingPlanService.Plan(new CuttingPlanRequest(plate, preservePartOrder: true)); + var preserved = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, preservePartOrder: true)); Assert.Equal(CuttingPlanStatus.ConstraintConflict, preserved.Status); var finding = Assert.Single(preserved.Findings); Assert.Same(host, finding.SourcePart); @@ -143,7 +143,7 @@ public class CuttingDependencyTests // Same insert as above plus a remote rapid and scribe mark: material is unchanged, so // the inner-before-host prerequisite must not depend on whole-program bounds. var (plate, host, inner) = NestedPlate(Marked(Rectangle(2.7, 2.7, 0.6, 0.6))); - var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest(plate)); + var snapshot = CuttingPlanService.Capture(CuttingPlanRequest.ForPlate(plate)); Assert.Null(snapshot.Findings.FirstOrDefault()?.Message); Assert.True(inner.BoundingBox.Left < host.BoundingBox.Left); @@ -158,7 +158,7 @@ public class CuttingDependencyTests { var (plate, host, inner) = NestedPlate(Rectangle(3.6, 2.8, 0.8, 0.4)); - var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); Assert.Equal(CuttingPlanStatus.UnsupportedGeometry, result.Status); var finding = Assert.Single(result.Findings); @@ -177,7 +177,7 @@ public class CuttingDependencyTests plate.Parts.Add(host); plate.Parts.Add(inner); - var snapshot = CuttingPlanService.Capture(new CuttingPlanRequest(plate)); + var snapshot = CuttingPlanService.Capture(CuttingPlanRequest.ForPlate(plate)); Assert.Null(snapshot.Findings.FirstOrDefault()?.Message); Assert.Empty(snapshot.Dependencies.PrerequisitesOf(0)); diff --git a/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs b/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs index 46e4f0d..7560e56 100644 --- a/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs +++ b/OpenNest.Tests/CuttingPlanning/CuttingPlanCommitTests.cs @@ -21,7 +21,7 @@ public class CuttingPlanCommitTests Assert.Contains(PostVerificationAnalyzer.Analyze(nest, Vector.Zero).Findings, f => f.Kind == PostVerificationKind.RapidCrossing); - var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); Assert.Equal(parts, plate.Parts); // Planning never mutates. var commit = CuttingPlanService.Apply([result]); @@ -53,7 +53,7 @@ public class CuttingPlanCommitTests // 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)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, confirmedParameters: parameters)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); var proposal = Assert.Single(result.ProposedOrder); Assert.True(proposal.IsRegenerated); @@ -104,7 +104,7 @@ public class CuttingPlanCommitTests 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)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); switch (change) { @@ -149,7 +149,7 @@ public class CuttingPlanCommitTests public void Apply_MalformedLiveProgram_IsStaleInsteadOfThrowing() { var (_, plate, parts) = FixedPlate(); - var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); Assert.Equal(CuttingPlanStatus.Ready, result.Status); parts[1].Program.Codes = null!; var events = Watch(plate); @@ -194,8 +194,8 @@ public class CuttingPlanCommitTests fixedPlate.Parts.Add(shared); var results = new[] { - CuttingPlanService.Plan(new CuttingPlanRequest(regenerated, confirmedParameters: parameters)), - CuttingPlanService.Plan(new CuttingPlanRequest(fixedPlate)) + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(regenerated, confirmedParameters: parameters)), + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(fixedPlate)) }; Assert.All(results, r => Assert.Equal(CuttingPlanStatus.Ready, r.Status)); Assert.True(results[0].ProposedOrder[0].IsRegenerated); @@ -229,8 +229,8 @@ public class CuttingPlanCommitTests var (_, second, secondParts) = FixedPlate(); var results = new[] { - CuttingPlanService.Plan(new CuttingPlanRequest(first)), - CuttingPlanService.Plan(new CuttingPlanRequest(second)) + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(first)), + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(second)) }; secondParts[0].LeadInsLocked = true; var firstState = PlateCuttingState.Capture(first); @@ -253,9 +253,9 @@ public class CuttingPlanCommitTests var (_, third, thirdPart, thirdParameters) = RegeneratedPlate(Vector.Zero); var results = new[] { - CuttingPlanService.Plan(new CuttingPlanRequest(first)), - CuttingPlanService.Plan(new CuttingPlanRequest(second, confirmedParameters: parameters)), - CuttingPlanService.Plan(new CuttingPlanRequest(third, confirmedParameters: thirdParameters)) + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(first)), + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(second, confirmedParameters: parameters)), + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(third, confirmedParameters: thirdParameters)) }; Assert.All(results, r => Assert.Equal(CuttingPlanStatus.Ready, r.Status)); var states = new[] { first, second, third }.Select(p => PlateCuttingState.Capture(p)).ToArray(); @@ -287,7 +287,7 @@ public class CuttingPlanCommitTests public void Apply_CancelledBeforeCommit_ChangesNothing() { var (_, plate, parts) = FixedPlate(); - var result = CuttingPlanService.Plan(new CuttingPlanRequest(plate)); + var result = CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)); var state = PlateCuttingState.Capture(plate); var events = Watch(plate); using var cancel = new CancellationTokenSource(); @@ -308,8 +308,8 @@ public class CuttingPlanCommitTests var (_, second, secondParts) = FixedPlate(); var results = new[] { - CuttingPlanService.Plan(new CuttingPlanRequest(first)), - CuttingPlanService.Plan(new CuttingPlanRequest(second)) + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(first)), + CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(second)) }; var failure = new InvalidOperationException("view refresh"); var seen = new List(); @@ -339,8 +339,8 @@ public class CuttingPlanCommitTests CuttingPlanResult[] results = fault switch { "detached" => [CuttingPlanService.Plan(new CuttingPlanRequest(parts))], - "not-ready" => [CuttingPlanService.Plan(new CuttingPlanRequest(plate, preservePartOrder: true))], - "duplicate" => Enumerable.Repeat(CuttingPlanService.Plan(new CuttingPlanRequest(plate)), 2).ToArray(), + "not-ready" => [CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate, preservePartOrder: true))], + "duplicate" => Enumerable.Repeat(CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(plate)), 2).ToArray(), _ => [] }; if (fault == "detached") @@ -356,6 +356,14 @@ public class CuttingPlanCommitTests Assert.Equal((0, 0, 0), events()); } + [Fact] + public void Request_NullDetachedListStaysUnambiguousAndInvalid() + { + // A plate overload of the constructor made this existing call ambiguous (CS0121). + Assert.Equal(CuttingPlanStatus.InvalidInput, CuttingPlanService.Plan(new CuttingPlanRequest(null)).Status); + Assert.Equal(CuttingPlanStatus.InvalidInput, CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(null)).Status); + } + [Fact] public void Apply_AllPlatesWithEmptySentinel_KeepsSentinelAndPlateList() { @@ -370,7 +378,7 @@ public class CuttingPlanCommitTests var listChanges = 0; manager.PlateListChanged += (_, _) => listChanges++; var sentinelEvents = Watch(sentinel); - var results = nest.Plates.ToArray().Select(p => CuttingPlanService.Plan(new CuttingPlanRequest(p))).ToArray(); + var results = nest.Plates.ToArray().Select(p => CuttingPlanService.Plan(CuttingPlanRequest.ForPlate(p))).ToArray(); Assert.All(results, r => Assert.Equal(CuttingPlanStatus.Ready, r.Status)); Assert.Empty(results[1].ProposedOrder); diff --git a/docs/cutting-planner.md b/docs/cutting-planner.md index c33fd6e..e20674b 100644 --- a/docs/cutting-planner.md +++ b/docs/cutting-planner.md @@ -15,15 +15,15 @@ programs and settings are stable. Pass that snapshot to `Plan` on a worker; defaults to `Vector.Zero`, not a discovered controller position. ```csharp -var request = new CuttingPlanRequest(plate, startPoint: start, +var request = CuttingPlanRequest.ForPlate(plate, startPoint: start, confirmedParameters: parameters, expansionBudget: 20000, maxEntries: 16, preservePartOrder: false); var snapshot = CuttingPlanService.Capture(request, cancellationToken); var result = CuttingPlanService.Plan(snapshot, cancellationToken); ``` -- A plate-scoped request plans the plate's current parts and records its exact - state for `Apply`. A detached part list (`new CuttingPlanRequest(parts, ...)`) +- A plate-scoped request (`CuttingPlanRequest.ForPlate`) plans the plate's current + parts and records its exact state for `Apply`. A detached part list (`new CuttingPlanRequest(parts, ...)`) plans the same way but can never be applied. An empty plate is a Ready no-op. - Omitting `confirmedParameters` preserves the original fixed-program contract: locked and unlocked programs stay fixed; only whole-part order may change.