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") } // ============================================================ diff --git a/pkg/ladder/allocate.go b/pkg/ladder/allocate.go index 116837d8e..021b65e01 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,131 @@ 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, + maxActions int, + layerStates map[LayerType]LayerState, + dataSources []string, +) ([]Allocation, []PlannedAction, []PlannedAction) { + if len(allocs)+len(reshapes) <= maxActions { + return allocs, reshapes, nil + } + // 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 := maxActions - len(reshapes) + kept, holds := truncatePurchases(allocs, purchaseCap, maxActions, dataSources) + return kept, reshapes, holds +} + +// truncateReshapes keeps the most urgent (lowest utilization) reshapes up to +// maxActions, emitting an ActionHold for each dropped one. Split from +// truncateToMaxActions to keep cyclomatic complexity within the project limit. +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 { + 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[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)", maxActions, r.Layer), + DataSources: dataSources, + }) + } + return sorted[:maxActions], 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..b3d7f836b 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,228 @@ 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 !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) + } + } +} + +// 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, 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() + 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) + } + // 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)", + cfg.MaxActionsPerRun, total, len(result.Allocations), len(result.Reshapes)) + } + // 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) + } +} + +// 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"}, } - if !strings.Contains(err.Error(), "max_actions_per_run") { - t.Errorf("error missing 'max_actions_per_run': %v", err) + 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") { + 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 != 3 { + t.Errorf("want 3 truncation holds (one per dropped purchase), got %d", truncHolds) + } +} + +// 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) + } } }