diff --git a/go.mod b/go.mod index 5fb3a2f6..abcf4fb6 100644 --- a/go.mod +++ b/go.mod @@ -59,7 +59,7 @@ require ( go.opentelemetry.io/otel/metric v1.45.0 // indirect go.opentelemetry.io/otel/trace v1.45.0 // indirect golang.org/x/crypto v0.58.0 - golang.org/x/net v0.61.0 // indirect + golang.org/x/net v0.62.0 // indirect golang.org/x/oauth2 v0.37.0 golang.org/x/sync v0.24.0 golang.org/x/sys v0.49.0 // indirect diff --git a/go.sum b/go.sum index 06deac0b..83df52e6 100644 --- a/go.sum +++ b/go.sum @@ -367,8 +367,8 @@ golang.org/x/crypto v0.58.0 h1:COkYLr4k7nDI4r6QtMML2AfzeIswWHAJthLS6K/M/54= golang.org/x/crypto v0.58.0/go.mod h1:Xh+kl5A+M0gMiWDytH2Hhr5Z7WbNCpv/eB/EFgfSk+w= golang.org/x/mod v0.42.0 h1:ICeL3t2L30ljSPHHxDnR4fqVcBt/zR2zJAb9juavmQg= golang.org/x/mod v0.42.0/go.mod h1:u5fzIrRInkL8MaO1cbRkUHT64v303YIT5KEqpNr7jRs= -golang.org/x/net v0.61.0 h1:RR+6j/BTBCrggOLugxa+66R8FvoN6MHlzDlZIbN9YFw= -golang.org/x/net v0.61.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= +golang.org/x/net v0.62.0 h1:18ELalfM5+XNRwzM2nYqXsZQ9ByS1uhj2O/U7+5jqRY= +golang.org/x/net v0.62.0/go.mod h1:WgDCOTH8iwtB66T4/GfI94AHo/hhdb7Hah9NDseQfiI= golang.org/x/oauth2 v0.37.0 h1:JUlcxA8oAtauLfiH8FX2/FkAWHAdi0QtGCGc+hofE98= golang.org/x/oauth2 v0.37.0/go.mod h1:IxwZNxUULJmpBFf9K/9NTMSIfZZuvuTy1gGxhigP/58= golang.org/x/sync v0.24.0 h1:iqiDrGoDgyuvAKrhLQthUEzU/R378N78gXbBjxed7oA= diff --git a/internal/config/store_postgres_ladder_amend_ceiling.go b/internal/config/store_postgres_ladder_amend_ceiling.go index 125b3357..0a067f2e 100644 --- a/internal/config/store_postgres_ladder_amend_ceiling.go +++ b/internal/config/store_postgres_ladder_amend_ceiling.go @@ -31,8 +31,10 @@ func toLadderMicros(v float64) int64 { return int64(math.Round(v * ladderMicros) // is netted per row in on-demand-equivalent dollars, the unit of the ceiling; // the amended row's old and new amounts convert by its own basis and rate. // Ceiling C = min(target, baseline * current coverage%) - existing, from the -// run's snapshot (existing is gross, so C errs low) and the config's current -// coverage. A run without a full snapshot refuses a raise. Must run inside +// run's snapshot and the config's current coverage. New runs store existing net +// of commitments expiring during the plan; older snapshots retain their gross +// value and therefore impose a stricter ceiling until the next run. A run +// without a full snapshot refuses a raise. Must run inside // withLadderConfigLock so the sums cannot move under it. func checkLadderConfigCeiling(ctx context.Context, tx pgx.Tx, configID string, amended lockedLadderTranche, newAmount string) error { var raise bool diff --git a/internal/server/handler_ladder.go b/internal/server/handler_ladder.go index c2e17a53..22a72f00 100644 --- a/internal/server/handler_ladder.go +++ b/internal/server/handler_ladder.go @@ -456,7 +456,7 @@ func (p *ladderRunPlanner) plan(rows []config.LadderInFlightRow) (*config.Ladder if err != nil { return nil, nil, err } - plan := assembleLadderPlan(p.engineCfg.Scope, p.now, allocResult, immediate, p.layerStates, p.baseline, p.engineCfg.TargetCoveragePct, p.term, p.paymentOpt) + plan := assembleLadderPlan(p.engineCfg.Scope, p.now, allocResult, immediate, p.baseline, p.engineCfg.TargetCoveragePct, p.term, p.paymentOpt) p.allocations, p.tranches, p.holds = len(allocResult.Allocations), len(trancheResult.Tranches), len(allocResult.Holds) return p.buildRows(runID, plan, trancheResult) } @@ -601,20 +601,22 @@ func ladderConfigToEngine(dbCfg *config.LadderConfigDB, accountID string) (pkgla // The monetary snapshot is derived to stay consistent with the authoritative // allocation rather than recomputed independently: // - target = baseline low-water * targetCoveragePct/100 (the coverage goal), -// - existing = sum of per-layer ExistingUSDPerHour, already converted to -// on-demand-equivalent dollars by ladderLayerStatesOnDemand, +// - existing = Allocate's NetExistingUSDPerHour, the net (existing minus +// expiring) commitment over the configured layers that the gap was sized +// against, in the on-demand-equivalent dollars ladderLayerStatesOnDemand +// converted the layer states to. Reporting the gross per-layer sum instead +// would show a gap that does not reconcile with target - existing, // - gap = sum of the planned allocations' on-demand-equivalent gaps (the unit // of target and existing), NOT target - existing. // // When the baseline low-water is nil the run is a Hold-only no-op (Allocate // returns no allocations), so target and gap stay nil (never 0-coerced); -// existing is still reported from the observed layer states. +// existing is still reported, since Allocate sets it on every result. func assembleLadderPlan( scope pkgladder.Scope, now time.Time, allocResult *pkgladder.AllocateResult, immediate map[pkgladder.LayerType]*big.Rat, - layerStates map[pkgladder.LayerType]pkgladder.LayerState, baseline pkgladder.UsageBaseline, targetCoveragePct float64, term pkgladder.Term, @@ -646,15 +648,12 @@ func assembleLadderPlan( // Holds are informational; append last. actions = append(actions, allocResult.Holds...) - // existing = sum of per-layer on-demand-equivalent existing coverage - // (always reportable). - var existingTotal float64 - for _, ls := range layerStates { - if ls.ExistingUSDPerHour != nil { - existingTotal += *ls.ExistingUSDPerHour - } + // existing is the net figure Allocate sized the gap against (#243), copied + // so the plan does not alias the result. + var existing *big.Rat + if allocResult.NetExistingUSDPerHour != nil { + existing = new(big.Rat).Set(allocResult.NetExistingUSDPerHour) } - existing := ratFromFloat(existingTotal) // target and gap are only meaningful when the baseline is available; nil // stays nil (a Hold-only no-op has no purchase target/gap to report). diff --git a/internal/server/ladder_existing_units_test.go b/internal/server/ladder_existing_units_test.go index c3886439..faa6d222 100644 --- a/internal/server/ladder_existing_units_test.go +++ b/internal/server/ladder_existing_units_test.go @@ -164,6 +164,51 @@ func TestExecuteLadderRun_ExpiringCoverageIsOnDemandEquivalent(t *testing.T) { assert.InDelta(t, 3.0, store.savedRun.TotalHourlyCommit, 1e-9) } +// #243: the run reports the net existing coverage Allocate sized the gap +// against, not the gross sum. Existing 4 and expiring 2 commitment at rate 0.5 +// is 8 OD gross, 4 OD net; target 10 - existing 4 = gap 6 must reconcile in +// ladder_runs and in the plan JSON the approval email is built from, on an +// allocating run and on a hold-only run alike. +func TestExecuteLadderRun_ReportsNetExistingCoverage(t *testing.T) { + cases := []struct { + name string + baseline pkgladder.UsageBaseline + wantGap bool + }{ + {"allocating run", testBaselineLowWater(12.5), true}, + {"baseline-unavailable hold", pkgladder.UsageBaseline{LookbackDays: 30, Percentile: 5}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := &ladderTestStore{} + cap := &fakeLadderCapability{t: t, baseline: tc.baseline, + rates: map[pkgladder.LayerType]float64{pkgladder.LayerConvertibleRI: 0.5, pkgladder.LayerEC2InstanceSP: 0.5}, + layerStates: map[pkgladder.LayerType]pkgladder.LayerState{ + pkgladder.LayerConvertibleRI: {Layer: pkgladder.LayerConvertibleRI, ExistingUSDPerHour: f64(0), ExpiringUSDPerHour: f64(0)}, + pkgladder.LayerEC2InstanceSP: spState(f64(4), f64(2)), + }} + require.NoError(t, runWithRates(t, store, cap)) + + run := store.savedRun + require.NotNil(t, run) + require.NotNil(t, run.ExistingUSDHr) + assert.InDelta(t, 4.0, *run.ExistingUSDHr, 1e-9, "existing_usd_hr is net of expiring, not the gross 8") + + var dto ladderPlanJSONDTO + require.NoError(t, json.Unmarshal(run.Plan, &dto)) + require.NotNil(t, dto.ExistingUSDHr) + assert.InDelta(t, 4.0, *dto.ExistingUSDHr, 1e-9, "plan JSON existing is the net figure") + if !tc.wantGap { + assert.Nil(t, run.GapUSDHr) + return + } + require.NotNil(t, run.GapUSDHr) + require.NotNil(t, run.TargetUSDHr) + assert.InDelta(t, *run.TargetUSDHr-*run.ExistingUSDHr, *run.GapUSDHr, 1e-9, "gap reconciles with target - existing") + }) + } +} + // Existing or expiring commitment on a layer with no rate cannot be // converted. Every such layer is reported, in layer order, with no partial // conversion for Allocate; layers that hold nothing still need no rate.