From cfb97a244b3d18a6e041cb311926911499a07069 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 11:46:55 +0300 Subject: [PATCH 1/3] fix(ladder): truncate lowest-priority allocations at max_actions_per_run Replace the permanent-stall error when a plan exceeds MaxActionsPerRun with deterministic truncation (B3 / Q-TRUNC). Reshapes are always retained first (risk-reducing, no new spend). Among purchase allocations the largest GapUSDPerHour are kept; the smallest are dropped. Every dropped action emits an ActionHold with a "deferred to a later run: max_actions_per_run=N reached" rationale so no money action is silently lost. Ties broken by layer name for a stable ordering. Implementation extracts truncateReshapes and truncatePurchases helpers to keep cyclomatic complexity under the project limit (gocyclo -over 10 clean). --- pkg/ladder/allocate.go | 131 ++++++++++++++++++++++++++++++--- pkg/ladder/allocate_test.go | 142 ++++++++++++++++++++++++++++++++++-- 2 files changed, 257 insertions(+), 16 deletions(-) diff --git a/pkg/ladder/allocate.go b/pkg/ladder/allocate.go index 116837d8e..f50aa569b 100644 --- a/pkg/ladder/allocate.go +++ b/pkg/ladder/allocate.go @@ -103,7 +103,9 @@ func (r *AllocateResult) IsNoOp() bool { // 6. Split gap across base, flex, and buffer roles. // 7. Apply per-run spend cap (proportional scaling), then drop sub-minimum // allocations with an informational hold each (never silently lost). -// 8. Enforce MaxActionsPerRun (error if exceeded). +// 8. Truncate to MaxActionsPerRun: sort purchases by GapUSDPerHour descending, +// keep the largest, emit one ActionHold per dropped action for auditability +// (reshapes are always retained first; no truncated money action is silently lost). func Allocate(in *AllocationInput) (*AllocateResult, error) { if in == nil { return nil, fmt.Errorf("allocation input must not be nil") @@ -448,17 +450,13 @@ func buildResult(in *AllocationInput, lowWater, target, existingTotal, gap *big. return nil, err } allocs, skipHolds := dropSubMinimumAllocations(allocs, in.DataSources) - totalActions := len(allocs) + len(reshapes) - if totalActions > in.Config.MaxActionsPerRun { - return nil, fmt.Errorf( - "plan exceeds max_actions_per_run (%d): %d allocations + %d reshapes = %d actions; adjust config or reduce scope", - in.Config.MaxActionsPerRun, len(allocs), len(reshapes), totalActions, - ) - } + allocs, reshapes, truncHolds := truncateToMaxActions(allocs, reshapes, in.Config.MaxActionsPerRun, in.LayerStates, in.DataSources) + holds := append(maintHolds, skipHolds...) + holds = append(holds, truncHolds...) return &AllocateResult{ Allocations: allocs, Reshapes: reshapes, - Holds: append(maintHolds, skipHolds...), + Holds: holds, }, nil } @@ -486,6 +484,121 @@ func dropSubMinimumAllocations(allocs []Allocation, dataSources []string) ([]All return kept, holds } +// truncateToMaxActions deterministically reduces allocs and reshapes to at +// most max total actions. Reshapes are preferred over purchases (they are +// risk-reducing with no new spend). Among purchases, the largest GapUSDPerHour +// are kept; the smallest are dropped. Every dropped action is returned as an +// ActionHold so no money action is silently lost. +// +// Priority rules (per Q-TRUNC): +// 1. Reshapes are always retained first. If reshapes alone exceed max, +// keep the most urgent (lowest utilization in layerStates; nil = treated +// as 100 so it is deprioritized), hold the rest. +// 2. Remaining slots (max - len(reshapes)) go to purchases sorted by +// GapUSDPerHour descending; ties broken by layer name for stable ordering. +func truncateToMaxActions( + allocs []Allocation, + reshapes []PlannedAction, + max int, + layerStates map[LayerType]LayerState, + dataSources []string, +) ([]Allocation, []PlannedAction, []PlannedAction) { + if len(allocs)+len(reshapes) <= max { + return allocs, reshapes, nil + } + // Case: reshapes alone fill or exceed the cap. + if len(reshapes) >= max { + kept, holds := truncateReshapes(reshapes, max, layerStates, dataSources) + return nil, kept, holds + } + // Normal case: reshapes fit; truncate the lowest-gap purchases. + purchaseCap := max - len(reshapes) + kept, holds := truncatePurchases(allocs, purchaseCap, max, dataSources) + return kept, reshapes, holds +} + +// truncateReshapes keeps the most urgent (lowest utilization) reshapes up to +// max, emitting an ActionHold for each dropped one. Split from +// truncateToMaxActions to keep cyclomatic complexity within the project limit. +func truncateReshapes(reshapes []PlannedAction, max int, layerStates map[LayerType]LayerState, dataSources []string) ([]PlannedAction, []PlannedAction) { + sorted := make([]PlannedAction, len(reshapes)) + copy(sorted, reshapes) + slices.SortStableFunc(sorted, func(a, b PlannedAction) int { + au := layerUtilPctOrMax(layerStates, a.Layer) + bu := layerUtilPctOrMax(layerStates, b.Layer) + if au < bu { + return -1 + } + if au > bu { + return 1 + } + if a.Layer < b.Layer { + return -1 + } + if a.Layer > b.Layer { + return 1 + } + return 0 + }) + var holds []PlannedAction + for _, r := range sorted[max:] { + holds = append(holds, PlannedAction{ + Action: ActionHold, + Layer: r.Layer, + Rationale: fmt.Sprintf("deferred to a later run: max_actions_per_run=%d reached (dropped reshape on %s)", max, r.Layer), + DataSources: dataSources, + }) + } + return sorted[:max], holds +} + +// truncatePurchases keeps the largest-GapUSDPerHour allocations up to +// purchaseCap, emitting an ActionHold for each dropped one. Split from +// truncateToMaxActions to keep cyclomatic complexity within the project limit. +func truncatePurchases(allocs []Allocation, purchaseCap, maxActions int, dataSources []string) ([]Allocation, []PlannedAction) { + sorted := make([]Allocation, len(allocs)) + copy(sorted, allocs) + slices.SortStableFunc(sorted, func(a, b Allocation) int { + // Descending by GapUSDPerHour (largest first). + cmp := b.GapUSDPerHour.Cmp(a.GapUSDPerHour) + if cmp != 0 { + return cmp + } + // Stable tie-break by layer name. + if a.Layer < b.Layer { + return -1 + } + if a.Layer > b.Layer { + return 1 + } + return 0 + }) + var holds []PlannedAction + for _, a := range sorted[purchaseCap:] { + holds = append(holds, PlannedAction{ + Action: ActionHold, + Layer: a.Layer, + Rationale: fmt.Sprintf( + "deferred to a later run: max_actions_per_run=%d reached (dropped %s on %s)", + maxActions, fmtRatUSD(a.GapUSDPerHour), a.Layer, + ), + DataSources: dataSources, + }) + } + return sorted[:purchaseCap], holds +} + +// layerUtilPctOrMax returns the UtilizationPct for the given layer from +// layerStates, or 100 when absent or nil. Treating unknown utilization as +// 100 (fully healthy) deprioritizes it when sorting reshapes by urgency, +// so confirmed low-utilization layers are retained over unmeasured ones. +func layerUtilPctOrMax(states map[LayerType]LayerState, layer LayerType) float64 { + if st, ok := states[layer]; ok && st.UtilizationPct != nil { + return *st.UtilizationPct + } + return 100 +} + // splitGap distributes gap across the base, buffer, and flex roles. It handles // the Azure merged-layer case where a single layer carries both RoleBase and // RoleBuffer. When BufferFraction > 0 but no layer carries RoleBuffer, it diff --git a/pkg/ladder/allocate_test.go b/pkg/ladder/allocate_test.go index a5bb072bb..53fb0a0e0 100644 --- a/pkg/ladder/allocate_test.go +++ b/pkg/ladder/allocate_test.go @@ -557,10 +557,20 @@ func TestAllocate_CapScaling(t *testing.T) { } } -func TestAllocate_MaxActionsExceeded(t *testing.T) { +// TestAllocate_MaxActionsExceeded_Truncates is the primary regression test for +// B3 / Q-TRUNC. Pre-fix: Allocate returned an error when the plan exceeded +// MaxActionsPerRun; the config would stall permanently. Post-fix: it truncates, +// keeps the largest-gap purchases, and emits ActionHold entries for dropped +// ones so no money action is silently lost. +// +// Scenario: 3-layer AWS setup (EC2InstanceSP=$4/hr, ComputeSP=$5/hr, +// ConvertibleRI=$1/hr), MaxActionsPerRun=2. The engine must return exactly 2 +// allocations (the two largest: ComputeSP and EC2InstanceSP) and one hold for +// the dropped ConvertibleRI allocation. +func TestAllocate_MaxActionsExceeded_Truncates(t *testing.T) { t.Parallel() cfg := validConfigAWS() - cfg.MaxActionsPerRun = 2 // 3 allocations will exceed this + cfg.MaxActionsPerRun = 2 // 3 allocations would normally be produced layers := awsLayers() in := &AllocationInput{ Config: cfg, @@ -568,13 +578,131 @@ func TestAllocate_MaxActionsExceeded(t *testing.T) { Baseline: UsageBaseline{LowWaterUSDPerHour: ptr(10.0), StableUSDPerHour: ptr(4.0)}, LayerStates: zeroStates(layers), Now: nowFixed(), + DataSources: []string{"cost-explorer"}, } - _, err := Allocate(in) - if err == nil { - t.Fatal("expected error for max_actions_per_run exceeded, got nil") + result, err := Allocate(in) + // Regression: pre-fix this returned an error; post-fix must succeed. + if err != nil { + t.Fatalf("expected no error after truncation fix, got: %v", err) + } + // Exactly MaxActionsPerRun money actions returned. + total := len(result.Allocations) + len(result.Reshapes) + if total != cfg.MaxActionsPerRun { + t.Errorf("want %d money actions, got %d (allocs=%d reshapes=%d)", + cfg.MaxActionsPerRun, total, len(result.Allocations), len(result.Reshapes)) + } + // Largest gaps kept: ComputeSP ($5/hr) and EC2InstanceSP ($4/hr). + kept := make(map[LayerType]bool, len(result.Allocations)) + for _, a := range result.Allocations { + kept[a.Layer] = true } - if !strings.Contains(err.Error(), "max_actions_per_run") { - t.Errorf("error missing 'max_actions_per_run': %v", err) + if !kept[LayerComputeSP] { + t.Error("ComputeSP ($5/hr gap, largest) must be kept") + } + if !kept[LayerEC2InstanceSP] { + t.Error("EC2InstanceSP ($4/hr gap, second largest) must be kept") + } + if kept[LayerConvertibleRI] { + t.Error("ConvertibleRI ($1/hr gap, smallest) must be dropped") + } + // Dropped allocation must produce an auditable hold, not be silently lost. + var truncHolds []PlannedAction + for _, h := range result.Holds { + if strings.Contains(h.Rationale, "max_actions_per_run") { + truncHolds = append(truncHolds, h) + } + } + if len(truncHolds) == 0 { + t.Error("expected at least one ActionHold for the dropped allocation") + } + for _, h := range truncHolds { + if h.Action != ActionHold { + t.Errorf("dropped allocation hold must have ActionHold, got %q", h.Action) + } + if !strings.Contains(h.Rationale, "max_actions_per_run=2") { + t.Errorf("hold rationale must contain max_actions_per_run=2: %q", h.Rationale) + } + if !strings.Contains(h.Rationale, "deferred to a later run") { + t.Errorf("hold rationale must contain 'deferred to a later run': %q", h.Rationale) + } + } +} + +// TestAllocate_MaxActions_ReshapeRetained verifies that when reshapes and +// purchases together exceed MaxActionsPerRun, reshapes are preferred over +// purchases: the reshape is kept, all purchase slots go to the largest-gap +// allocations, and the smallest purchase is dropped with a hold. +func TestAllocate_MaxActions_ReshapeRetained(t *testing.T) { + t.Parallel() + cfg := validConfigAWS() + cfg.MaxActionsPerRun = 2 // 2 slots: 1 reshape + 1 purchase (out of 3 allocs) + layers := awsLayers() + // ConvertibleRI has existing commitments with low utilization -> reshape. + states := zeroStates(layers) + states = withExisting(states, LayerConvertibleRI, 5.0) + states = withBufferUtilization(states, 50.0) // below 90% threshold -> reshape + in := &AllocationInput{ + Config: cfg, + Layers: layers, + Baseline: UsageBaseline{LowWaterUSDPerHour: ptr(10.0), StableUSDPerHour: ptr(4.0)}, + LayerStates: states, + Now: nowFixed(), + DataSources: []string{"cost-explorer"}, + } + result, err := Allocate(in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Must not exceed MaxActionsPerRun. + total := len(result.Allocations) + len(result.Reshapes) + if total > cfg.MaxActionsPerRun { + t.Errorf("want <= %d money actions, got %d (allocs=%d reshapes=%d)", + cfg.MaxActionsPerRun, total, len(result.Allocations), len(result.Reshapes)) + } + // The reshape must be retained. + if len(result.Reshapes) == 0 { + t.Error("reshape must be retained when reshapes are preferred over purchases") + } + // At least one hold for a dropped purchase must be present. + var truncHolds int + for _, h := range result.Holds { + if strings.Contains(h.Rationale, "max_actions_per_run") { + truncHolds++ + } + } + if truncHolds == 0 { + t.Error("expected at least one truncation hold for a dropped purchase") + } +} + +// TestAllocate_ExactCap_Untouched verifies that a plan whose action count +// exactly equals MaxActionsPerRun is returned unmodified with no truncation holds. +func TestAllocate_ExactCap_Untouched(t *testing.T) { + t.Parallel() + cfg := validConfigAWS() + cfg.MaxActionsPerRun = 3 // exactly the number of allocations produced + layers := awsLayers() + in := &AllocationInput{ + Config: cfg, + Layers: layers, + Baseline: UsageBaseline{LowWaterUSDPerHour: ptr(10.0), StableUSDPerHour: ptr(4.0)}, + LayerStates: zeroStates(layers), + Now: nowFixed(), + DataSources: []string{"cost-explorer"}, + } + result, err := Allocate(in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // All 3 allocations must be kept at exact-cap. + if len(result.Allocations) != 3 { + t.Errorf("want 3 allocations at exact cap, got %d", len(result.Allocations)) + } + // No truncation holds expected. + for _, h := range result.Holds { + if strings.Contains(h.Rationale, "max_actions_per_run") { + t.Errorf("unexpected truncation hold at exact cap: %q", h.Rationale) + } } } From 70b5d28f9db7edd604021fc35234630b36099bf2 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 11:56:12 +0300 Subject: [PATCH 2/3] fix(ladder): emit audit holds for purchases dropped when reshapes fill the cap When reshapes alone fill or exceed MaxActionsPerRun, truncateToMaxActions previously returned nil allocations while emitting holds only for dropped reshapes, so every purchase vanished with zero audit trace. This violated the Q-TRUNC rule that every dropped money action must leave a Hold. That branch now converts all allocations to holds (truncatePurchases with purchaseCap 0), naming each dropped layer and amount. Also: document the branch invariant (unreachable in production: at most one buffer reshape and MaxActionsPerRun > 0), rename the shadowing max parameter to maxActions in the truncate helpers, and tighten tests to set-level assertions (exact surviving/held layers). Adds TestAllocate_MaxActions_ReshapesFillCap as the regression for the dropped-purchase-hold gap. --- pkg/ladder/allocate.go | 36 +++++++---- pkg/ladder/allocate_test.go | 123 ++++++++++++++++++++++++++++++++---- 2 files changed, 133 insertions(+), 26 deletions(-) diff --git a/pkg/ladder/allocate.go b/pkg/ladder/allocate.go index f50aa569b..021b65e01 100644 --- a/pkg/ladder/allocate.go +++ b/pkg/ladder/allocate.go @@ -499,28 +499,38 @@ func dropSubMinimumAllocations(allocs []Allocation, dataSources []string) ([]All func truncateToMaxActions( allocs []Allocation, reshapes []PlannedAction, - max int, + maxActions int, layerStates map[LayerType]LayerState, dataSources []string, ) ([]Allocation, []PlannedAction, []PlannedAction) { - if len(allocs)+len(reshapes) <= max { + if len(allocs)+len(reshapes) <= maxActions { return allocs, reshapes, nil } - // Case: reshapes alone fill or exceed the cap. - if len(reshapes) >= max { - kept, holds := truncateReshapes(reshapes, max, layerStates, dataSources) - return nil, kept, holds + // Case: reshapes alone fill or exceed the cap. Every dropped purchase must + // still leave an audit Hold (Q-TRUNC), so ALL allocations are converted to + // holds here via purchaseCap 0; none is silently discarded. + // + // INVARIANT: this branch is currently unreachable in production. + // validateRoleCounts caps buffer layers at 1 (so at most one reshape) and + // MaxActionsPerRun is validated > 0, so len(reshapes) < maxActions always + // holds. truncateReshapes' utilization sort and layerUtilPctOrMax's + // nil->100 path are therefore defensive-only, kept for future multi-buffer + // topologies. + if len(reshapes) >= maxActions { + keptReshapes, reshapeHolds := truncateReshapes(reshapes, maxActions, layerStates, dataSources) + _, allocHolds := truncatePurchases(allocs, 0, maxActions, dataSources) + return nil, keptReshapes, append(allocHolds, reshapeHolds...) } // Normal case: reshapes fit; truncate the lowest-gap purchases. - purchaseCap := max - len(reshapes) - kept, holds := truncatePurchases(allocs, purchaseCap, max, dataSources) + purchaseCap := maxActions - len(reshapes) + kept, holds := truncatePurchases(allocs, purchaseCap, maxActions, dataSources) return kept, reshapes, holds } // truncateReshapes keeps the most urgent (lowest utilization) reshapes up to -// max, emitting an ActionHold for each dropped one. Split from +// maxActions, emitting an ActionHold for each dropped one. Split from // truncateToMaxActions to keep cyclomatic complexity within the project limit. -func truncateReshapes(reshapes []PlannedAction, max int, layerStates map[LayerType]LayerState, dataSources []string) ([]PlannedAction, []PlannedAction) { +func truncateReshapes(reshapes []PlannedAction, maxActions int, layerStates map[LayerType]LayerState, dataSources []string) ([]PlannedAction, []PlannedAction) { sorted := make([]PlannedAction, len(reshapes)) copy(sorted, reshapes) slices.SortStableFunc(sorted, func(a, b PlannedAction) int { @@ -541,15 +551,15 @@ func truncateReshapes(reshapes []PlannedAction, max int, layerStates map[LayerTy return 0 }) var holds []PlannedAction - for _, r := range sorted[max:] { + for _, r := range sorted[maxActions:] { holds = append(holds, PlannedAction{ Action: ActionHold, Layer: r.Layer, - Rationale: fmt.Sprintf("deferred to a later run: max_actions_per_run=%d reached (dropped reshape on %s)", max, r.Layer), + Rationale: fmt.Sprintf("deferred to a later run: max_actions_per_run=%d reached (dropped reshape on %s)", maxActions, r.Layer), DataSources: dataSources, }) } - return sorted[:max], holds + return sorted[:maxActions], holds } // truncatePurchases keeps the largest-GapUSDPerHour allocations up to diff --git a/pkg/ladder/allocate_test.go b/pkg/ladder/allocate_test.go index 53fb0a0e0..b3d7f836b 100644 --- a/pkg/ladder/allocate_test.go +++ b/pkg/ladder/allocate_test.go @@ -628,10 +628,30 @@ func TestAllocate_MaxActionsExceeded_Truncates(t *testing.T) { } } +// truncatedHoldLayers returns the set of layers named by max_actions_per_run +// truncation holds in the result, so tests can assert exactly which actions +// were dropped (not merely how many). +func truncatedHoldLayers(holds []PlannedAction) map[LayerType]bool { + out := make(map[LayerType]bool, len(holds)) + for _, h := range holds { + if strings.Contains(h.Rationale, "max_actions_per_run") { + out[h.Layer] = true + } + } + return out +} + // TestAllocate_MaxActions_ReshapeRetained verifies that when reshapes and // purchases together exceed MaxActionsPerRun, reshapes are preferred over -// purchases: the reshape is kept, all purchase slots go to the largest-gap -// allocations, and the smallest purchase is dropped with a hold. +// purchases: the reshape is kept, the single remaining slot goes to the +// largest-gap purchase, and every dropped purchase leaves a hold. +// +// Fixture: lowWater=10, stable=4, buffer_fraction=0.10, ConvertibleRI existing +// $5/hr @ 50% util (-> reshape). gap = min(10-5, 10-5) = 5; coreGap = 4.5; +// baseGap = min(4.5, stable 4) = 4.0 -> EC2InstanceSP; bufferGap = 0.5 -> +// ConvertibleRI; flexGap = 0.5 -> ComputeSP. So the largest purchase is +// EC2InstanceSP ($4/hr); ComputeSP and ConvertibleRI ($0.5/hr each) are dropped. +// With cap=2 (1 reshape + 1 purchase), EC2InstanceSP survives. func TestAllocate_MaxActions_ReshapeRetained(t *testing.T) { t.Parallel() cfg := validConfigAWS() @@ -653,25 +673,102 @@ func TestAllocate_MaxActions_ReshapeRetained(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - // Must not exceed MaxActionsPerRun. + // Exactly MaxActionsPerRun money actions: 1 reshape + 1 purchase. total := len(result.Allocations) + len(result.Reshapes) - if total > cfg.MaxActionsPerRun { - t.Errorf("want <= %d money actions, got %d (allocs=%d reshapes=%d)", + if total != cfg.MaxActionsPerRun { + t.Errorf("want %d money actions, got %d (allocs=%d reshapes=%d)", cfg.MaxActionsPerRun, total, len(result.Allocations), len(result.Reshapes)) } - // The reshape must be retained. - if len(result.Reshapes) == 0 { - t.Error("reshape must be retained when reshapes are preferred over purchases") + // Exactly one reshape, on the buffer layer (ConvertibleRI), retained. + if len(result.Reshapes) != 1 || result.Reshapes[0].Layer != LayerConvertibleRI { + t.Fatalf("want exactly one reshape on ConvertibleRI, got %+v", result.Reshapes) + } + // Exactly one surviving purchase, on the largest-gap layer (EC2InstanceSP). + if len(result.Allocations) != 1 { + t.Fatalf("want exactly one surviving purchase, got %d", len(result.Allocations)) + } + if result.Allocations[0].Layer != LayerEC2InstanceSP { + t.Errorf("surviving purchase must be the largest-gap layer EC2InstanceSP, got %s", + result.Allocations[0].Layer) + } + // Exactly the two smaller purchases were dropped with holds. + held := truncatedHoldLayers(result.Holds) + wantHeld := map[LayerType]bool{LayerComputeSP: true, LayerConvertibleRI: true} + if !maps.Equal(held, wantHeld) { + t.Errorf("dropped-purchase hold layers = %v, want %v", held, wantHeld) } - // At least one hold for a dropped purchase must be present. +} + +// TestAllocate_MaxActions_ReshapesFillCap is the regression for the CRITICAL +// Fable finding: when reshapes alone fill (>=) the cap, every dropped purchase +// must still leave an audit Hold. Pre-fix, this branch discarded all +// allocations with ZERO holds (holds=0); post-fix each dropped purchase names +// its layer + amount in a max_actions_per_run hold. +// +// Fixture matches TestAllocate_MaxActions_ReshapeRetained but with cap=1: the +// single reshape (ConvertibleRI) consumes the only slot, so all three purchases +// (EC2InstanceSP $4/hr, ComputeSP $0.5/hr, ConvertibleRI $0.5/hr) are dropped. +func TestAllocate_MaxActions_ReshapesFillCap(t *testing.T) { + t.Parallel() + cfg := validConfigAWS() + cfg.MaxActionsPerRun = 1 // one slot, taken by the reshape + layers := awsLayers() + states := zeroStates(layers) + states = withExisting(states, LayerConvertibleRI, 5.0) + states = withBufferUtilization(states, 50.0) // below 90% threshold -> reshape + in := &AllocationInput{ + Config: cfg, + Layers: layers, + Baseline: UsageBaseline{LowWaterUSDPerHour: ptr(10.0), StableUSDPerHour: ptr(4.0)}, + LayerStates: states, + Now: nowFixed(), + DataSources: []string{"cost-explorer"}, + } + result, err := Allocate(in) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Exactly one money action, the retained reshape; no purchases survive. + total := len(result.Allocations) + len(result.Reshapes) + if total != cfg.MaxActionsPerRun { + t.Errorf("want %d money action, got %d (allocs=%d reshapes=%d)", + cfg.MaxActionsPerRun, total, len(result.Allocations), len(result.Reshapes)) + } + if len(result.Allocations) != 0 { + t.Errorf("no purchase may survive when the reshape fills the cap, got %d", len(result.Allocations)) + } + if len(result.Reshapes) != 1 || result.Reshapes[0].Layer != LayerConvertibleRI { + t.Fatalf("want the ConvertibleRI reshape retained, got %+v", result.Reshapes) + } + // CRITICAL: every dropped purchase must leave an audit hold (pre-fix: 0). + held := truncatedHoldLayers(result.Holds) + wantHeld := map[LayerType]bool{ + LayerEC2InstanceSP: true, + LayerComputeSP: true, + LayerConvertibleRI: true, + } + if !maps.Equal(held, wantHeld) { + t.Errorf("dropped-purchase hold layers = %v, want %v", held, wantHeld) + } + // Each truncation hold must name its layer and the dropped amount. var truncHolds int for _, h := range result.Holds { - if strings.Contains(h.Rationale, "max_actions_per_run") { - truncHolds++ + if !strings.Contains(h.Rationale, "max_actions_per_run") { + continue + } + truncHolds++ + if h.Action != ActionHold { + t.Errorf("truncation hold must be ActionHold, got %q", h.Action) + } + if !strings.Contains(h.Rationale, string(h.Layer)) { + t.Errorf("hold rationale must name its layer %s: %q", h.Layer, h.Rationale) + } + if !strings.Contains(h.Rationale, "/hr") { + t.Errorf("hold rationale must name the dropped amount: %q", h.Rationale) } } - if truncHolds == 0 { - t.Error("expected at least one truncation hold for a dropped purchase") + if truncHolds != 3 { + t.Errorf("want 3 truncation holds (one per dropped purchase), got %d", truncHolds) } } From b7bc832f64c89e2e40f5516fc17970f417fc63e9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 16:12:52 +0300 Subject: [PATCH 3/3] test(server): update MaxActionsExceeded handler test to the truncate contract PR #1362 added TestExecuteLadderRun_MaxActionsExceeded asserting the old engine contract (Allocate errors when the split exceeds MaxActionsPerRun). This branch changes that contract to deterministic truncation, so it owns the consumer-test update. Renamed to TestExecuteLadderRun_MaxActionsExceeded_TruncatesAndPersists and rewrote it to the new contract: executeLadderRun succeeds, persists a status=planned run whose plan contains exactly one surviving purchase (the larger-gap $70/hr flex allocation) plus a max_actions_per_run hold for the dropped $10/hr base allocation, TotalHourlyCommit reflects only the kept allocation, and tranches exist for it. The fixture ramp moves to a single future step (AfterDays=30) since an AfterDays=0 step becomes a BuyNow action and produces no tranche rows. --- internal/server/handler_ladder_test.go | 71 ++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 11 deletions(-) diff --git a/internal/server/handler_ladder_test.go b/internal/server/handler_ladder_test.go index 77a206245..8dfa4bfe4 100644 --- a/internal/server/handler_ladder_test.go +++ b/internal/server/handler_ladder_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "strings" "testing" "time" @@ -622,21 +623,34 @@ func TestExecuteLadderRun_ZeroGap_HoldPlan(t *testing.T) { assert.Empty(t, store.savedTranches, "a zero-gap Hold run produces no tranches") } -// TestExecuteLadderRun_MaxActionsExceeded covers Allocate erroring because the -// split produces more allocations than MaxActionsPerRun allows. executeLadderRun -// must fail loud and persist nothing. -func TestExecuteLadderRun_MaxActionsExceeded(t *testing.T) { +// TestExecuteLadderRun_MaxActionsExceeded_TruncatesAndPersists covers the +// engine's truncation contract at MaxActionsPerRun: when the split produces +// more allocations than the cap, Allocate no longer errors. It keeps the +// largest-gap allocations, converts each dropped one into an ActionHold +// ("deferred to a later run"), and executeLadderRun persists the truncated +// plan normally. The over-cap config keeps running every cadence instead of +// stalling forever. +func TestExecuteLadderRun_MaxActionsExceeded_TruncatesAndPersists(t *testing.T) { ctx := testutil.TestContext(t) now := time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC) dbCfg := validTestDBConfig("cfg-maxactions") dbCfg.MaxActionsPerRun = 1 // base+flex split yields 2 allocations > 1 + // A future-only ramp step so the kept allocation lands as a scheduled + // tranche row (an AfterDays==0 step becomes a BuyNow action instead, + // which produces no ladder_tranches rows). + rampJSON, err := json.Marshal(pkgladder.RampSchedule{ + Steps: []pkgladder.RampStep{{AfterDays: 30, Fraction: 1.0}}, + }) + require.NoError(t, err) + dbCfg.RampSchedule = rampJSON store := &ladderTestStore{} app := &Application{Config: store} // Stable (10) is far below the target (80% of low-water 100 = 80), so the - // base layer only absorbs up to stable and the remainder spills to flex, - // producing TWO allocations. With MaxActionsPerRun=1 the engine must error. + // base layer absorbs up to stable (10) and the remainder (70) spills to + // flex, producing TWO allocations. With MaxActionsPerRun=1 the engine + // keeps the larger flex allocation and holds the smaller base one. cap := &fakeLadderCapability{ t: t, baseline: pkgladder.UsageBaseline{ @@ -647,11 +661,46 @@ func TestExecuteLadderRun_MaxActionsExceeded(t *testing.T) { }, } - err := app.executeLadderRun(ctx, &dbCfg, cap, "123456789012", pkgladder.Term1Year, pkgladder.PaymentNoUpfront, now) - require.Error(t, err, "Allocate must error when the split exceeds max_actions_per_run") - assert.Contains(t, err.Error(), "max_actions_per_run") - assert.Nil(t, store.savedRun, "no run may be persisted when Allocate errors") - assert.Nil(t, store.savedTranches, "no tranches may be persisted when Allocate errors") + err = app.executeLadderRun(ctx, &dbCfg, cap, "123456789012", pkgladder.Term1Year, pkgladder.PaymentNoUpfront, now) + require.NoError(t, err, "over-cap plans must truncate, not error (permanent-stall regression)") + + require.NotNil(t, store.savedRun, "the truncated run must be persisted") + run := store.savedRun + assert.Equal(t, pkgladder.RunStatusPlanned, run.Status) + + var planDTO ladderPlanJSONDTO + require.NoError(t, json.Unmarshal(run.Plan, &planDTO)) + + // Exactly ONE purchase survives: the larger-gap flex allocation ($70/hr on + // EC2InstanceSP, the fake's RoleFlex layer); the $10/hr base allocation is + // dropped. At least one hold must record the truncation for audit. + var purchases []ladderPlanJSONAction + var truncHolds []ladderPlanJSONAction + for _, a := range planDTO.Actions { + switch a.Action { + case string(pkgladder.ActionPurchase): + purchases = append(purchases, a) + case string(pkgladder.ActionHold): + if strings.Contains(a.Rationale, "max_actions_per_run=1") { + truncHolds = append(truncHolds, a) + } + } + } + require.Len(t, purchases, 1, "exactly MaxActionsPerRun purchase actions must survive") + assert.Equal(t, string(pkgladder.LayerEC2InstanceSP), purchases[0].Layer, + "the largest-gap (flex, $70/hr) allocation must be the survivor") + require.NotNil(t, purchases[0].AmountUSDHr) + assert.InDelta(t, 70.0, *purchases[0].AmountUSDHr, 1e-9) + require.NotEmpty(t, truncHolds, "each dropped allocation must leave a max_actions_per_run hold") + assert.Contains(t, truncHolds[0].Rationale, "deferred to a later run") + + // TotalHourlyCommit counts only the KEPT allocation ($70/hr), not the full + // pre-truncation gap ($80/hr). + assert.InDelta(t, 70.0, run.TotalHourlyCommit, 1e-9, + "total_hourly_commit must reflect the truncated plan only") + + // Tranches exist for the kept allocation. + assert.NotEmpty(t, store.savedTranches, "the kept allocation must produce tranches") } // ============================================================