fix(release): enforce one finish per commit and keep real unplaced reasons

Delta review fixes:

- ReleaseRenest throws when a commit does not call finish, or calls it
  twice: either would leave ledger plates uncoded or count a sheet twice
  for later nests of the run. The ledger sheet count is the codes given.
- A stage whose layout is not kept now lists the copies its layout
  missed with their own reasons (a part no sheet fits stays "does not
  fit"); only the copies it placed are "the proposal cannot be kept".
- A valid ledger layout that places nothing says "Auto Nest placed
  nothing on it" rather than claiming no part fits.
- ReleaseLedgerOffer rejects null rows; docs list every ledger note.
This commit is contained in:
aj committed 2026-11-03 18:03:24 -05:00
1 parent 9efeb4e5b6
commit be5fef6875
6 files changed
+121 -29

No files matched your search

@@ -19,7 +19,8 @@ public sealed record ReleaseLedgerOffer(IReadOnlyList<LedgerStockRow> Rows, stri
/// <summary>
/// The rows as engine stock: finite, under their <c>ledger:{n}</c> ids, no costs.
/// </summary>
public IReadOnlyList<PlateOption> Options { get; } = Rows
public IReadOnlyList<PlateOption> Options { get; } =
(Rows ?? throw new ArgumentNullException(nameof(Rows)))
.Select(row => new PlateOption
{
Width = row.Size.Width,
@@ -132,24 +133,23 @@ public sealed class ReleaseLedgerStock
}
/// <summary>
/// The release note for <paramref name="nest"/> nested on <paramref name="offer"/>: how
/// many sheets came from the ledger, or why it was not used.
/// The release note for a nest nested on <paramref name="offer"/>: how many sheets came
/// from the ledger, or why it was not used.
/// </summary>
/// <param name="sheets">The ledger codes <see cref="AssignCodes"/> gave the nest.</param>
/// <param name="layoutNotKept">
/// True when Auto Nest could not keep a layout on the offer.
/// </param>
public static string Note(ReleaseLedgerOffer offer, Nest nest, bool layoutNotKept)
public static string Note(ReleaseLedgerOffer offer, int sheets, bool layoutNotKept)
{
ArgumentNullException.ThrowIfNull(offer);
ArgumentNullException.ThrowIfNull(nest);
if (offer.NotUsed != null)
return $"MaterialLedger not used: {offer.NotUsed}.";
var sheets = nest.Plates.Sum(p => p.StockItemCodes.Count);
var note = layoutNotKept
? "MaterialLedger not used: Auto Nest could not keep a layout on its stock."
: sheets == 0
? "No MaterialLedger stock used: no part fits it."
? "No MaterialLedger stock used: Auto Nest placed nothing on it."
: string.Format(CultureInfo.CurrentCulture,
"{0} sheet{1} from MaterialLedger stock.", sheets, sheets == 1 ? "" : "s");
return offer.OtherNestsUnchecked == null
@@ -296,4 +296,51 @@ public class ReleaseRenestTests
Assert.Equal(["S-1"], codesAtClose);
}
/// <summary>Rectangles, except that a ledger sheet gets an empty layout.</summary>
private sealed class NothingOnLedger : INestingEngine
{
private readonly INestingEngine _inner = NestingEngineRegistry.Create("Rectangles");
public NestJobResult Solve(NestJob job, IProgress<NestJobProgress> progress = null,
CancellationToken token = default) =>
LedgerStockMapper.IsLedgerId(job.Plates[0].Id)
? new NestJobResultBuilder(job).Build(NestJobStopReason.Completed)
: _inner.Solve(job, progress, token);
}
[Fact]
public async Task AValidLedgerLayoutThatPlacesNothing_SaysSo()
{
var ledger = new FakeLedger { Stock = Stock(Sheet("S-1", 60, 120)) };
var nest = NestOf(20, 2);
var result = await Release(ledger.Create(), nest, new NothingOnLedger());
Assert.True(result.Nested, result.Note);
Assert.EndsWith(" No MaterialLedger stock used: Auto Nest placed nothing on it.",
result.Note);
}
[Theory]
[InlineData(0)]
[InlineData(2)]
public async Task ACommitThatDoesNotFinishItsPlatesOnce_Throws(int finishes)
{
var ledger = new FakeLedger { Stock = Stock(Sheet("S-1", 60, 120)) }.Create();
var nest = NestOf(20, 2);
await Assert.ThrowsAsync<InvalidOperationException>(() => ReleaseRenest.RunAsync(nest,
ledger, "Rectangles", 0,
request => Task.FromResult(NestPipeline.Run(NestingEngineRegistry.Create("Rectangles"),
"Rectangles", request, null, CancellationToken.None)),
(result, finish) =>
{
var plates = NestPipelineCommit.ApplyToPlates(result, nest.CreatePlate);
for (var i = 0; i < finishes; i++)
finish(plates);
return plates;
},
CancellationToken.None));
}
}
+26 -12
View File
@@ -25,8 +25,8 @@ namespace OpenNest.Desktop
{
/// <summary>
/// Attaches a kept result's sheets to the nest's plates and returns them. It must call
/// <c>finish</c> with those plates before its undo step closes; that gives them their
/// ledger codes.
/// <c>finish</c> once with those plates before its undo step closes; that gives them
/// their ledger codes. Not calling it, or calling it twice, throws.
/// </summary>
public delegate IReadOnlyList<Plate> Commit(NestPipelineResult result,
Action<IReadOnlyList<Plate>> finish);
@@ -47,24 +47,38 @@ namespace OpenNest.Desktop
var wanted = nest.Drawings.Any(d => !d.IsCutOff && d.Quantity.Remaining > 0);
var offer = ledger != null && wanted ? await ledger.OfferAsync(nest, token) : null;
var locations = new Dictionary<string, string>(StringComparer.Ordinal);
var sheets = 0;
var renested = await NestRenest.RunStagedAsync(nest, engineName, offer?.Options,
minimumSalvageDimension, solve,
result => commit(result, plates =>
result =>
{
if (offer == null)
return;
var stock = NestPipelineCommit.PairWithStock(result, plates)
.Select(p => (p.Plate, p.Stock.Id))
.ToList();
foreach (var (code, location) in ledger.AssignCodes(nest, offer, stock))
locations[code] = location;
}),
var finished = false;
var plates = commit(result, applied =>
{
if (finished)
throw new InvalidOperationException(
"A release commit finished its plates twice.");
finished = true;
if (offer == null)
return;
var stock = NestPipelineCommit.PairWithStock(result, applied)
.Select(p => (p.Plate, p.Stock.Id))
.ToList();
foreach (var (code, location) in ledger.AssignCodes(nest, offer, stock))
locations[code] = location;
sheets += stock.Sum(p => p.Plate.StockItemCodes.Count);
});
if (!finished)
throw new InvalidOperationException(
"A release commit did not finish its plates, so they have no codes.");
return plates;
},
token);
var ledgerNote = offer == null
? null
: ReleaseLedgerStock.Note(offer, nest, renested.FirstStockNotKept);
: ReleaseLedgerStock.Note(offer, sheets, renested.FirstStockNotKept);
return new ReleaseRenestResult(renested.Nested, Join(renested.Note, ledgerNote),
locations);
}
+20 -7
View File
@@ -9,7 +9,10 @@ namespace OpenNest.Engine.Jobs;
/// <summary>
/// What <see cref="NestRenest"/> kept: whether a layout was committed, its physical sheets
/// and utilization, the copies it left unplaced, and one operator-facing note.
/// and utilization, the copies it left unplaced, and one operator-facing note. A staged run
/// (<see cref="NestRenest.RunStagedAsync"/>) lists every copy of a layout it did not keep,
/// the ones that layout placed as <see cref="NestUnplacedReason.ProposalNotKeepable"/>;
/// <see cref="NestRenest.Keep"/> lists the result's own unplaced copies.
/// </summary>
public sealed record NestRenestResult(
bool Nested,
@@ -126,7 +129,7 @@ public static class NestRenest
if (!result.IsValid || !result.CanKeep)
{
// Every copy this stage asked for is left, not just what its layout missed.
unplaced = NotKept(request);
unplaced = NotKept(request, result);
firstStockNotKept |= stock != null;
}
continue;
@@ -212,11 +215,21 @@ public static class NestRenest
.Select(p => (p.Plate, p.Stock.Id))
.ToList();
private static IReadOnlyList<NestUnplacedPart> NotKept(NestPipelineRequest request) =>
request.Items
.Select(item => new NestUnplacedPart(item.Drawing.Name, item.Drawing, item.Quantity,
NestUnplacedReason.ProposalNotKeepable))
.ToList();
// The copies the layout missed keep their own reasons; the ones it placed were not kept.
private static IReadOnlyList<NestUnplacedPart> NotKept(NestPipelineRequest request,
NestPipelineResult result)
{
var unplaced = result.Unplaced.ToList();
foreach (var item in request.Items)
{
var listed = unplaced.Where(u => ReferenceEquals(u.Drawing, item.Drawing))
.Sum(u => u.Count);
if (item.Quantity > listed)
unplaced.Add(new NestUnplacedPart(item.Drawing.Name, item.Drawing,
item.Quantity - listed, NestUnplacedReason.ProposalNotKeepable));
}
return unplaced;
}
private static bool IsKeepable(NestPipelineResult result) =>
result.IsValid && result.CanKeep && result.Plates.Any(p => p.Parts.Count > 0);
+16
View File
@@ -122,6 +122,22 @@ public class NestRenestTests
Assert.StartsWith("Nested on 1 sheet", kept.Note);
}
[Fact]
public void ANotKeptLayout_KeepsTheRealReasonForCopiesNoSheetFits()
{
var nest = NestOf(("SYN-A", 4, 2, 2), ("SYN-HUGE", 200, 200, 1));
var result = NestRenest.Run(nest, new OverlappingEngine(), "overlapping");
Assert.False(result.Nested);
Assert.Equal(
[
("SYN-A", 2, NestUnplacedReason.ProposalNotKeepable),
("SYN-HUGE", 1, NestUnplacedReason.NoStockFits),
],
result.Unplaced.Select(u => (u.Drawing.Name, u.Count, u.Reason)).OrderBy(u => u.Name));
}
/// <summary>Puts every copy at the same spot, so the layout never validates.</summary>
private sealed class OverlappingEngine : INestingEngine
{
+5 -3
View File
@@ -729,10 +729,12 @@ every drawing's required quantity with the selected engine, in two steps:
When the nest has no material or thickness, or the ledger is not set up on the
PC, cannot be reached, rejects the API key, has no mapping for the material or
has no usable stock of it (the note counts what was left out: on other nests,
taken by earlier nests of this release, too small for any part), the group goes
taken by earlier nests of this release, too small for any part, of unknown
shape; "no free stock" when the ledger had none at all), the group goes
straight to step 2 and its note says "MaterialLedger not used: " and why. So
does a step 1 whose layout Auto Nest cannot keep. Otherwise the note says how
many sheets came from MaterialLedger stock, and adds "Codes on other nests were
does a step 1 whose layout Auto Nest cannot keep. A step 1 that places nothing
says "No MaterialLedger stock used: Auto Nest placed nothing on it." Otherwise
the note says how many sheets came from MaterialLedger stock, and adds "Codes on other nests were
not checked" with the reason when the nest server could not list them. The
ledger is not read for a group with nothing to nest.