From 1e5b4fee841b357cb0984d5816384006881023e4 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 11:47:34 +0300 Subject: [PATCH 1/4] fix(aws): stale-series detection in on-demand baseline (G6a) Introduce two named-constant guards in GetUsageBaseline that fail loud when the CE on-demand series cannot be trusted: maxMissingDays = 3 -- coverage check: series must have >= lookbackDays-3 points (CE lags ~24-48 h; up to 3 absent days ok) maxSeriesAgeDays = 3 -- freshness check: most-recent DailyPoint must be within 3 calendar days of now; older means the feed stalled or the date range is wrong Both conditions return an explicit error (not a zeroed/nil baseline) so the engine config is counted Errored and surfaced to the operator rather than silently trusting a stale low-water clamp. Interface change: onDemandSeriesSource.GetOnDemandSeries now returns []DailyPoint{Date, USDPerHour} instead of []float64, giving GetUsageBaseline the Date field it needs for the freshness check. L2 (real CE wiring) will implement this updated signature. Float64 values are extracted via extractValues() before passing to the unchanged validateSeries / nearestRankPercentile helpers. Regression tests added: TestGetUsageBaseline_StaleSeries_ReturnsError -- tail > maxSeriesAgeDays old TestGetUsageBaseline_SparseSeries_ReturnsError -- too few points for lookback TestGetUsageBaseline_FreshAndCompleteSeries_NoError -- happy path unbroken --- providers/aws/ladder/baseline.go | 80 ++++++++++++++++- providers/aws/ladder/interfaces.go | 26 ++++-- providers/aws/ladder/ladder_test.go | 130 +++++++++++++++++++++++----- 3 files changed, 205 insertions(+), 31 deletions(-) diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 36c474290..8bc006d77 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -5,6 +5,7 @@ import ( "fmt" "math" "sort" + "time" "github.com/LeanerCloud/CUDly/pkg/ladder" ) @@ -15,6 +16,24 @@ import ( // The caller (engine) should configure LookbackDays >= minBaselineSeriesDays. const minBaselineSeriesDays = 7 +// maxMissingDays is the maximum number of calendar days that may be absent +// from the series within the configured lookback window and still yield a +// trustworthy baseline. CE data typically lags 24-48 hours, so a 30-day +// window may return 28-29 points; one extra day of tolerance is provided. +// A series with more than maxMissingDays absent days returns a coverage error. +// Distinct from minBaselineSeriesDays: this is a window-relative completeness +// check, not an absolute floor. +const maxMissingDays = 3 + +// maxSeriesAgeDays is the maximum age in calendar days of the most-recent data +// point in the on-demand series before the series is considered stale. +// CE cost data lags 24-48 hours, so yesterday's data (1 day old) is normal; +// a point older than maxSeriesAgeDays indicates the CE feed has stalled, the +// date-range parameter is wrong, or CE has temporarily stopped ingesting data +// for the account. GetUsageBaseline fails loud rather than computing a baseline +// from stale data that the utilization clamp would then trust. +const maxSeriesAgeDays = 3 + // GetUsageBaseline computes a statistical low-water-mark from a daily // on-demand-equivalent USD/hour series returned by the injected onDemandSeriesSource. // @@ -44,6 +63,10 @@ const minBaselineSeriesDays = 7 // - Series empty: hard error (no data from the coverage source). // - Series shorter than minBaselineSeriesDays: hard error (insufficient // history for a reliable percentile estimate). +// - Series with too few points for the lookback window (more than +// maxMissingDays gaps): hard error (coverage check). +// - Most-recent point older than maxSeriesAgeDays days: hard error (stale +// series; GetUsageBaseline fails loud rather than trusting stale data). // - Series containing a non-finite (NaN/Inf) or negative element: hard error // naming the offending index (a cost series must be finite and >= 0). // - percentile not in (0, 100]: hard error (out-of-range). @@ -55,19 +78,36 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo return ladder.UsageBaseline{}, err } - series, err := a.onDemand.GetOnDemandSeries(ctx, a.cfg.Region, lookbackDays) + points, err := a.onDemand.GetOnDemandSeries(ctx, a.cfg.Region, lookbackDays) if err != nil { return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series fetch failed: %w", err) } - if len(series) == 0 { + if len(points) == 0 { return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: on-demand series is empty for region %s (series source returned no data)", a.cfg.Region) } - if len(series) < minBaselineSeriesDays { + if len(points) < minBaselineSeriesDays { return ladder.UsageBaseline{}, fmt.Errorf( "GetUsageBaseline: series length %d is below minimum %d days; extend the lookback window or check the on-demand series source", - len(series), minBaselineSeriesDays, + len(points), minBaselineSeriesDays, + ) + } + // Coverage check: the series must span most of the requested lookback + // window. CE typically lags 24-48h, so up to maxMissingDays absent days + // is acceptable; more indicates truncated date ranges or feed gaps. + if minRequired := lookbackDays - maxMissingDays; len(points) < minRequired { + return ladder.UsageBaseline{}, fmt.Errorf( + "GetUsageBaseline: series has %d points for a %d-day lookback window (minimum %d = %d - maxMissingDays %d); the on-demand series source may have gaps or a truncated date range", + len(points), lookbackDays, minRequired, lookbackDays, maxMissingDays, ) } + // Freshness check: the most-recent point must be within maxSeriesAgeDays. + // A stale tail means the CE feed has stopped or the date range is wrong; + // computing a baseline from stale data would yield a confidently wrong clamp. + if fErr := validateSeriesFreshness(points); fErr != nil { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: %w", fErr) + } + + series := extractValues(points) if vErr := validateSeries(series); vErr != nil { return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: %w", vErr) } @@ -91,6 +131,38 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo }, nil } +// validateSeriesFreshness returns an error when the most-recent DailyPoint is +// older than maxSeriesAgeDays calendar days. It is called after the series has +// been confirmed non-empty and above minBaselineSeriesDays by GetUsageBaseline, +// so len(points) > 0 is guaranteed. +// +// Age is computed as integer days (floor of elapsed hours / 24), which is +// precise enough for a 3-day threshold and avoids DST and leap-second edge +// cases that a Date difference would introduce. +func validateSeriesFreshness(points []DailyPoint) error { + latestDate := points[len(points)-1].Date + ageDays := int(time.Since(latestDate).Hours() / 24) + if ageDays > maxSeriesAgeDays { + return fmt.Errorf( + "on-demand series is stale: most recent data point is %s (%d days old, maximum %d); check that the CE data feed is active or that the lookback date range ends near today", + latestDate.Format("2006-01-02"), ageDays, maxSeriesAgeDays, + ) + } + return nil +} + +// extractValues returns the USDPerHour field of each DailyPoint as a plain +// []float64, ordered oldest-to-newest, for use by validateSeries and +// nearestRankPercentile. The Date fields are consumed by validateSeriesFreshness +// and not needed downstream. +func extractValues(points []DailyPoint) []float64 { + out := make([]float64, len(points)) + for i, p := range points { + out[i] = p.USDPerHour + } + return out +} + // validateSeries rejects series containing non-finite (NaN/Inf) or negative // elements at the trust boundary: the series is injected via onDemandSeriesSource, // and a single bad element would silently corrupt the percentile (NaN makes diff --git a/providers/aws/ladder/interfaces.go b/providers/aws/ladder/interfaces.go index 67a0ed38b..02a56048e 100644 --- a/providers/aws/ladder/interfaces.go +++ b/providers/aws/ladder/interfaces.go @@ -77,16 +77,28 @@ type riCoverageSource interface { GetRICoverageMap(ctx context.Context, lookbackDays int, regions []string) (recommendations.PoolCoverageMap, error) } +// DailyPoint is a single calendar-day entry in the on-demand cost series. +// Date is the UTC calendar day (time-of-day component is irrelevant; callers +// should truncate to midnight UTC for consistent comparisons). +// USDPerHour is the average on-demand-equivalent spend in USD per hour for +// that calendar day (total daily spend / 24). +type DailyPoint struct { + // Date is the UTC calendar day this data point covers. + Date time.Time + // USDPerHour is the on-demand-equivalent spend averaged over the day. + USDPerHour float64 +} + // onDemandSeriesSource is the narrow interface for the daily on-demand spend // series consumed by GetUsageBaseline. GetOnDemandSeries returns a slice of -// len(lookbackDays) daily on-demand-equivalent USD/hour values for the given -// region, ordered oldest-to-newest. Each element is the average on-demand -// spend in USD per hour for that calendar day. The real implementation sources -// this from CE GetCostAndUsage with Granularity=Daily filtered to on-demand -// usage types; wiring happens when the cost-and-usage collector PR lands. -// Tests pass a hermetic fake. +// DailyPoints for the given region and lookback window, ordered oldest-to-newest. +// Each element covers one calendar day; the Date field allows GetUsageBaseline +// to enforce freshness (the most-recent point must be within maxSeriesAgeDays). +// The real implementation sources this from CE GetCostAndUsage with +// Granularity=Daily filtered to on-demand usage types; wiring happens when the +// cost-and-usage collector PR (L2) lands. Tests pass a hermetic fake. type onDemandSeriesSource interface { - GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]float64, error) + GetOnDemandSeries(ctx context.Context, region string, lookbackDays int) ([]DailyPoint, error) } // utilizationSource is the narrow interface for RI utilization data. diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index b0932d178..344149127 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -3,6 +3,7 @@ package ladder import ( "context" "errors" + "fmt" "math" "testing" "time" @@ -48,15 +49,15 @@ type fakeCoverageSource struct { coverageErr error onDemandErr error coverageMap recommendations.PoolCoverageMap - onDemandSeries []float64 + onDemandPoints []DailyPoint } func (f *fakeCoverageSource) GetRICoverageMap(_ context.Context, _ int, _ []string) (recommendations.PoolCoverageMap, error) { return f.coverageMap, f.coverageErr } -func (f *fakeCoverageSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]float64, error) { - return f.onDemandSeries, f.onDemandErr +func (f *fakeCoverageSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]DailyPoint, error) { + return f.onDemandPoints, f.onDemandErr } // fakeUtilizationSource: err field before utils for fieldalignment. @@ -703,8 +704,7 @@ func TestNearestRankPercentile(t *testing.T) { func TestGetUsageBaseline_SingleDaySeries_ReturnsThatValue(t *testing.T) { // A 7-day series with all same value; p5 should return 3.0. - series := []float64{3.0, 3.0, 3.0, 3.0, 3.0, 3.0, 3.0} - cov := &fakeCoverageSource{onDemandSeries: series} + cov := &fakeCoverageSource{onDemandPoints: makeConstantPoints(7, 3.0)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) bl, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) @@ -722,7 +722,7 @@ func TestGetUsageBaseline_SingleDaySeries_ReturnsThatValue(t *testing.T) { func TestGetUsageBaseline_P5OfVariedSeries(t *testing.T) { // 20-element series [1..20]; p5 nearest-rank: ceil(5/100*20)=ceil(1)=1 -> sorted[0]=1. - cov := &fakeCoverageSource{onDemandSeries: makeRange(20)} + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(20)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) bl, err := a.GetUsageBaseline(context.Background(), testScope(), 20, 5.0) @@ -732,7 +732,7 @@ func TestGetUsageBaseline_P5OfVariedSeries(t *testing.T) { } func TestGetUsageBaseline_EmptySeries_ReturnsError(t *testing.T) { - cov := &fakeCoverageSource{onDemandSeries: []float64{}} + cov := &fakeCoverageSource{onDemandPoints: []DailyPoint{}} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) require.Error(t, err) @@ -740,7 +740,8 @@ func TestGetUsageBaseline_EmptySeries_ReturnsError(t *testing.T) { } func TestGetUsageBaseline_SeriesTooShort_ReturnsError(t *testing.T) { - cov := &fakeCoverageSource{onDemandSeries: []float64{1.0, 2.0, 3.0}} // < 7 days + // 3 points with recent dates: fires minBaselineSeriesDays=7 check (3 < 7). + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(3)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), 7, 5.0) require.Error(t, err) @@ -748,9 +749,9 @@ func TestGetUsageBaseline_SeriesTooShort_ReturnsError(t *testing.T) { } func TestGetUsageBaseline_NaNElement_ReturnsErrorNamingIndex(t *testing.T) { - series := makeRange(10) - series[4] = math.NaN() - cov := &fakeCoverageSource{onDemandSeries: series} + points := makeRecentPoints(10) + points[4].USDPerHour = math.NaN() + cov := &fakeCoverageSource{onDemandPoints: points} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) require.Error(t, err, "a NaN element must be rejected at the boundary") @@ -759,9 +760,9 @@ func TestGetUsageBaseline_NaNElement_ReturnsErrorNamingIndex(t *testing.T) { } func TestGetUsageBaseline_InfElement_ReturnsErrorNamingIndex(t *testing.T) { - series := makeRange(10) - series[7] = math.Inf(1) - cov := &fakeCoverageSource{onDemandSeries: series} + points := makeRecentPoints(10) + points[7].USDPerHour = math.Inf(1) + cov := &fakeCoverageSource{onDemandPoints: points} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) require.Error(t, err, "an Inf element must be rejected at the boundary") @@ -770,9 +771,9 @@ func TestGetUsageBaseline_InfElement_ReturnsErrorNamingIndex(t *testing.T) { } func TestGetUsageBaseline_NegativeElement_ReturnsErrorNamingIndex(t *testing.T) { - series := makeRange(10) - series[2] = -0.5 - cov := &fakeCoverageSource{onDemandSeries: series} + points := makeRecentPoints(10) + points[2].USDPerHour = -0.5 + cov := &fakeCoverageSource{onDemandPoints: points} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), 10, 5.0) require.Error(t, err, "a negative cost element must be rejected at the boundary") @@ -789,8 +790,7 @@ func TestGetUsageBaseline_OnDemandSourceError_Propagates(t *testing.T) { } func TestGetUsageBaseline_InvalidPercentile_ReturnsError(t *testing.T) { - series := makeRange(30) - cov := &fakeCoverageSource{onDemandSeries: series} + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(30)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) for _, p := range []float64{0.0, -1.0, 101.0} { @@ -801,13 +801,72 @@ func TestGetUsageBaseline_InvalidPercentile_ReturnsError(t *testing.T) { } func TestGetUsageBaseline_WrongScope_ReturnsError(t *testing.T) { - cov := &fakeCoverageSource{onDemandSeries: makeRange(30)} + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(30)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) badScope := ladder.Scope{Provider: common.ProviderAWS, AccountID: "wrong"} _, err := a.GetUsageBaseline(context.Background(), badScope, 30, 5.0) require.Error(t, err) } +// TestGetUsageBaseline_StaleSeries_ReturnsError is a regression test for gap G6a: +// before stale-series detection was added, a CE feed that stopped days ago would +// yield a low-water baseline with no error, and the utilization clamp would trust +// it as though it were current. Now GetUsageBaseline must return an explicit error +// naming the last date when the most-recent point is older than maxSeriesAgeDays. +func TestGetUsageBaseline_StaleSeries_ReturnsError(t *testing.T) { + // Build a 30-point series whose most-recent point is maxSeriesAgeDays+1 days + // in the past, simulating a CE feed that stopped (or a wrong date range). + staleEnd := time.Now().AddDate(0, 0, -(maxSeriesAgeDays + 1)).UTC().Truncate(24 * time.Hour) + points := make([]DailyPoint, 30) + for i := range points { + points[i] = DailyPoint{ + Date: staleEnd.AddDate(0, 0, -(29 - i)), + USDPerHour: float64(i + 1), + } + } + cov := &fakeCoverageSource{onDemandPoints: points} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.Error(t, err, "a series whose tail is older than maxSeriesAgeDays must be rejected") + assert.Contains(t, err.Error(), "stale", "error must say 'stale'") + assert.Contains(t, err.Error(), staleEnd.Format("2006-01-02"), "error must name the most-recent date so callers can diagnose the CE feed gap") +} + +// TestGetUsageBaseline_SparseSeries_ReturnsError is a regression test for gap G6a: +// before coverage detection was added, a series with too many missing days for the +// requested lookback window would silently yield a percentile over incomplete data. +// Now GetUsageBaseline must return an error when the series has fewer than +// lookbackDays - maxMissingDays points. +func TestGetUsageBaseline_SparseSeries_ReturnsError(t *testing.T) { + const lookbackDays = 30 + // n is one point below the minimum: lookbackDays - maxMissingDays - 1. + // At n=26 (>= minBaselineSeriesDays=7) the coverage check fires, not the + // absolute-minimum check. + n := lookbackDays - maxMissingDays - 1 + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(n)} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) + require.Error(t, err, "a series with too many gaps for the lookback window must be rejected") + assert.Contains(t, err.Error(), fmt.Sprintf("%d points", n), "error must report the actual point count") + assert.Contains(t, err.Error(), fmt.Sprintf("%d-day lookback", lookbackDays), "error must report the requested lookback") +} + +// TestGetUsageBaseline_FreshAndCompleteSeries_NoError is a regression test +// confirming that the stale-series guard does not break the happy path: +// a fresh, sufficiently complete series must still yield a valid baseline. +func TestGetUsageBaseline_FreshAndCompleteSeries_NoError(t *testing.T) { + // 30 points ending yesterday, which is well within maxSeriesAgeDays=3 and + // covers the 30-day lookback window with only one day of natural lag. + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(30)} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + bl, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.NoError(t, err, "a fresh, complete series must pass all stale-series checks") + require.NotNil(t, bl.LowWaterUSDPerHour, "LowWaterUSDPerHour must be set on a valid baseline") +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- @@ -820,3 +879,34 @@ func makeRange(n int) []float64 { } return out } + +// makeRecentPoints returns n DailyPoints whose USDPerHour values match +// makeRange(n) (values 1.0...float64(n)), with dates ending yesterday. +// Yesterday is used because CE data typically lags ~24 h; ending at yesterday +// keeps the series well within the maxSeriesAgeDays freshness window while +// being realistic for a real CE feed. Ordered oldest-to-newest. +func makeRecentPoints(n int) []DailyPoint { + yesterday := time.Now().AddDate(0, 0, -1).UTC().Truncate(24 * time.Hour) + out := make([]DailyPoint, n) + for i := range out { + out[i] = DailyPoint{ + Date: yesterday.AddDate(0, 0, -(n - 1 - i)), + USDPerHour: float64(i + 1), + } + } + return out +} + +// makeConstantPoints returns n DailyPoints all with the given USDPerHour value, +// ending yesterday. Ordered oldest-to-newest. +func makeConstantPoints(n int, value float64) []DailyPoint { + yesterday := time.Now().AddDate(0, 0, -1).UTC().Truncate(24 * time.Hour) + out := make([]DailyPoint, n) + for i := range out { + out[i] = DailyPoint{ + Date: yesterday.AddDate(0, 0, -(n - 1 - i)), + USDPerHour: value, + } + } + return out +} From cf7ecc1f39388f4d0ee8817427cfccb923c8d367 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 11:56:14 +0300 Subject: [PATCH 2/4] fix(aws): validate strictly-increasing dates before baseline freshness check Fable review hardening for the stale-series guard. validateSeriesFreshness trusted the documented oldest-to-newest ordering by reading the last element as the newest point. A future L2 CE adapter returning unsorted or duplicate-date pages could make a stale tail pass (fresh point last) or a fresh series falsely error. Add validateSeriesChronology: reject any non-monotonic or duplicate calendar date (compared after UTC-day truncation) with an explicit error naming the offending index and dates. Runs before the age check, so the last element is provably the latest. It also catches duplicate-date rows the count-based coverage check cannot see (N points spanning < N days). Boundary tests lock both off-by-ones: TestGetUsageBaseline_AgeExactlyAtBoundary_NoError -- age == max passes TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError -- 27 points pass TestGetUsageBaseline_NonMonotonicDates_ReturnsError -- swap + dup date --- providers/aws/ladder/baseline.go | 38 +++++++++++++++++-- providers/aws/ladder/ladder_test.go | 59 +++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 8bc006d77..0a3d60dc4 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -131,15 +131,26 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo }, nil } -// validateSeriesFreshness returns an error when the most-recent DailyPoint is -// older than maxSeriesAgeDays calendar days. It is called after the series has -// been confirmed non-empty and above minBaselineSeriesDays by GetUsageBaseline, -// so len(points) > 0 is guaranteed. +// validateSeriesFreshness returns an error when the series is not in strictly +// increasing calendar-day order or when its most-recent DailyPoint is older +// than maxSeriesAgeDays calendar days. It is called after the series has been +// confirmed non-empty and above minBaselineSeriesDays by GetUsageBaseline, so +// len(points) > 0 is guaranteed. +// +// The chronology check comes first and fails loud on any non-monotonic or +// duplicate calendar date: the freshness check trusts the last element to be +// the newest point, so an unsorted or duplicate-date series (e.g. from a future +// CE adapter that returns pages out of order) could otherwise make a stale tail +// pass or a fresh series falsely error. It also catches duplicate-date rows the +// count-based coverage check cannot see (N points spanning fewer than N days). // // Age is computed as integer days (floor of elapsed hours / 24), which is // precise enough for a 3-day threshold and avoids DST and leap-second edge // cases that a Date difference would introduce. func validateSeriesFreshness(points []DailyPoint) error { + if err := validateSeriesChronology(points); err != nil { + return err + } latestDate := points[len(points)-1].Date ageDays := int(time.Since(latestDate).Hours() / 24) if ageDays > maxSeriesAgeDays { @@ -151,6 +162,25 @@ func validateSeriesFreshness(points []DailyPoint) error { return nil } +// validateSeriesChronology verifies the series is in strictly increasing +// calendar-day order (oldest-to-newest, no duplicate days). Dates are compared +// after truncation to the UTC day so that a same-day pair with differing +// time-of-day components is still rejected as a duplicate. The error names the +// offending index and both dates so a data-source ordering bug is traceable. +func validateSeriesChronology(points []DailyPoint) error { + for i := 1; i < len(points); i++ { + prev := points[i-1].Date.UTC().Truncate(24 * time.Hour) + cur := points[i].Date.UTC().Truncate(24 * time.Hour) + if !cur.After(prev) { + return fmt.Errorf( + "on-demand series is not in strictly increasing date order: point %d (%s) does not come after point %d (%s); the series source must return one entry per day, oldest-to-newest", + i, points[i].Date.Format("2006-01-02"), i-1, points[i-1].Date.Format("2006-01-02"), + ) + } + } + return nil +} + // extractValues returns the USDPerHour field of each DailyPoint as a plain // []float64, ordered oldest-to-newest, for use by validateSeries and // nearestRankPercentile. The Date fields are consumed by validateSeriesFreshness diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index 344149127..82a3b4c1d 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -867,6 +867,65 @@ func TestGetUsageBaseline_FreshAndCompleteSeries_NoError(t *testing.T) { require.NotNil(t, bl.LowWaterUSDPerHour, "LowWaterUSDPerHour must be set on a valid baseline") } +// TestGetUsageBaseline_AgeExactlyAtBoundary_NoError locks the off-by-one on the +// freshness comparison: a series whose most-recent point is EXACTLY +// maxSeriesAgeDays old must pass (the check is age > max, not age >= max). +func TestGetUsageBaseline_AgeExactlyAtBoundary_NoError(t *testing.T) { + // End the series exactly maxSeriesAgeDays days ago. Truncate to the UTC day + // and step back one extra hour so integer-day age floors to exactly + // maxSeriesAgeDays rather than maxSeriesAgeDays-1 due to sub-day rounding. + end := time.Now().Add(-time.Duration(maxSeriesAgeDays)*24*time.Hour - time.Hour).UTC().Truncate(24 * time.Hour) + points := make([]DailyPoint, 30) + for i := range points { + points[i] = DailyPoint{Date: end.AddDate(0, 0, -(29 - i)), USDPerHour: float64(i + 1)} + } + cov := &fakeCoverageSource{onDemandPoints: points} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.NoError(t, err, "a series exactly maxSeriesAgeDays old must pass (boundary is age > max)") +} + +// TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError locks the off-by-one on +// the coverage comparison: exactly lookbackDays - maxMissingDays points must +// pass (the check is len < minRequired, not len <= minRequired). +func TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError(t *testing.T) { + const lookbackDays = 30 + n := lookbackDays - maxMissingDays // exactly the minimum required + cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(n)} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) + require.NoError(t, err, "exactly lookbackDays-maxMissingDays points must pass (boundary is len < minRequired)") +} + +// TestGetUsageBaseline_NonMonotonicDates_ReturnsError covers the chronology +// hardening: an out-of-order or duplicate-date series must fail loud rather than +// let validateSeriesFreshness trust the last element as newest. +func TestGetUsageBaseline_NonMonotonicDates_ReturnsError(t *testing.T) { + t.Run("swapped pair", func(t *testing.T) { + points := makeRecentPoints(30) + points[10], points[11] = points[11], points[10] // break strict ordering + cov := &fakeCoverageSource{onDemandPoints: points} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.Error(t, err, "an out-of-order series must be rejected") + assert.Contains(t, err.Error(), "strictly increasing") + }) + + t.Run("duplicate date", func(t *testing.T) { + points := makeRecentPoints(30) + points[15].Date = points[14].Date // duplicate calendar day + cov := &fakeCoverageSource{onDemandPoints: points} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), 30, 5.0) + require.Error(t, err, "a duplicate-date series must be rejected") + assert.Contains(t, err.Error(), "strictly increasing") + }) +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- From 69682a058506c28943c3c9a72ad92b008abe32c6 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 12:21:17 +0300 Subject: [PATCH 3/4] fix(aws): in-window coverage + calendar-day freshness in baseline Address CodeRabbit review on the stale-series guard (PR #1365): 1. Coverage check counted TOTAL points, so a series with enough points smeared across more than lookbackDays days (e.g. 27 points over 53 days) passed despite sparse in-window coverage. New validateInWindowCoverage counts only points whose date falls within [today-lookbackDays, today] and requires that count >= lookbackDays - maxMissingDays; fail-loud otherwise. 2. Freshness derived "days old" from elapsed hours (floor(hours/24)), making the result time-of-day dependent and contradicting the calendar-day contract. Age is now the whole-UTC-calendar-day difference between today and the latest point, both normalized via the new utcDay helper. today is computed once in GetUsageBaseline and shared by both the coverage window and the freshness check for consistent date math. 3. Test hardening: the freshness/stale/recent date helpers built dates via hour arithmetic before truncation, which selected the wrong calendar day between 00:00-00:59 UTC and intermittently failed. All date-based test fixtures now build UTC calendar days directly. New regression test: TestGetUsageBaseline_InsufficientInWindowCoverage_ReturnsError -- 27 total points, only 20 in-window -> error (fails on the old total-count check, passes now). --- providers/aws/ladder/baseline.go | 73 ++++++++++++++++++++++------- providers/aws/ladder/ladder_test.go | 64 ++++++++++++++++++------- 2 files changed, 103 insertions(+), 34 deletions(-) diff --git a/providers/aws/ladder/baseline.go b/providers/aws/ladder/baseline.go index 0a3d60dc4..be5649fb6 100644 --- a/providers/aws/ladder/baseline.go +++ b/providers/aws/ladder/baseline.go @@ -63,8 +63,9 @@ const maxSeriesAgeDays = 3 // - Series empty: hard error (no data from the coverage source). // - Series shorter than minBaselineSeriesDays: hard error (insufficient // history for a reliable percentile estimate). -// - Series with too few points for the lookback window (more than -// maxMissingDays gaps): hard error (coverage check). +// - Too few points INSIDE the lookback window (more than maxMissingDays +// missing days): hard error (coverage check; total count is not enough, +// the points must actually fall within [today-lookbackDays, today]). // - Most-recent point older than maxSeriesAgeDays days: hard error (stale // series; GetUsageBaseline fails loud rather than trusting stale data). // - Series containing a non-finite (NaN/Inf) or negative element: hard error @@ -91,19 +92,20 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo len(points), minBaselineSeriesDays, ) } - // Coverage check: the series must span most of the requested lookback - // window. CE typically lags 24-48h, so up to maxMissingDays absent days - // is acceptable; more indicates truncated date ranges or feed gaps. - if minRequired := lookbackDays - maxMissingDays; len(points) < minRequired { - return ladder.UsageBaseline{}, fmt.Errorf( - "GetUsageBaseline: series has %d points for a %d-day lookback window (minimum %d = %d - maxMissingDays %d); the on-demand series source may have gaps or a truncated date range", - len(points), lookbackDays, minRequired, lookbackDays, maxMissingDays, - ) + // today is normalized once to midnight UTC and shared by both the coverage + // window and the freshness check so all date math uses the same calendar-day + // reference (see utcDay). + today := utcDay(time.Now()) + // Coverage check: enough points must fall INSIDE the requested lookback + // window. CE typically lags 24-48h, so up to maxMissingDays absent days is + // acceptable; more indicates truncated date ranges or feed gaps. + if cErr := validateInWindowCoverage(points, lookbackDays, today); cErr != nil { + return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: %w", cErr) } // Freshness check: the most-recent point must be within maxSeriesAgeDays. // A stale tail means the CE feed has stopped or the date range is wrong; // computing a baseline from stale data would yield a confidently wrong clamp. - if fErr := validateSeriesFreshness(points); fErr != nil { + if fErr := validateSeriesFreshness(points, today); fErr != nil { return ladder.UsageBaseline{}, fmt.Errorf("GetUsageBaseline: %w", fErr) } @@ -131,6 +133,41 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo }, nil } +// utcDay normalizes t to the start of its UTC calendar day (midnight UTC). +// All freshness and lookback-window math compares dates as whole UTC calendar +// days; funneling every date through utcDay keeps those comparisons +// deterministic and independent of the current time-of-day. +func utcDay(t time.Time) time.Time { + return t.UTC().Truncate(24 * time.Hour) +} + +// validateInWindowCoverage rejects a series that does not have enough points +// falling INSIDE the lookback window [today-lookbackDays, today]. Counting the +// total number of points is insufficient: N points smeared across more than N +// days (e.g. 27 points spread over 53 days) would pass a naive count check +// while leaving the recent window sparsely covered, so the percentile would be +// computed over data that does not represent the requested window. Every date +// is normalized to its UTC calendar day (utcDay) before comparison. +// +// today must already be normalized via utcDay by the caller. +func validateInWindowCoverage(points []DailyPoint, lookbackDays int, today time.Time) error { + windowStart := today.AddDate(0, 0, -lookbackDays) + inWindow := 0 + for _, p := range points { + d := utcDay(p.Date) + if !d.Before(windowStart) && !d.After(today) { + inWindow++ + } + } + if minRequired := lookbackDays - maxMissingDays; inWindow < minRequired { + return fmt.Errorf( + "on-demand series has %d points inside the %d-day lookback window (minimum %d = %d - maxMissingDays %d); the series source may have gaps or a truncated/misaligned date range", + inWindow, lookbackDays, minRequired, lookbackDays, maxMissingDays, + ) + } + return nil +} + // validateSeriesFreshness returns an error when the series is not in strictly // increasing calendar-day order or when its most-recent DailyPoint is older // than maxSeriesAgeDays calendar days. It is called after the series has been @@ -144,19 +181,19 @@ func (a *AWSLadder) GetUsageBaseline(ctx context.Context, scope ladder.Scope, lo // pass or a fresh series falsely error. It also catches duplicate-date rows the // count-based coverage check cannot see (N points spanning fewer than N days). // -// Age is computed as integer days (floor of elapsed hours / 24), which is -// precise enough for a 3-day threshold and avoids DST and leap-second edge -// cases that a Date difference would introduce. -func validateSeriesFreshness(points []DailyPoint) error { +// Age is the difference in whole UTC calendar days between today and the +// most-recent point (both normalized via utcDay), so the result is +// time-of-day independent. today must already be normalized by the caller. +func validateSeriesFreshness(points []DailyPoint, today time.Time) error { if err := validateSeriesChronology(points); err != nil { return err } - latestDate := points[len(points)-1].Date - ageDays := int(time.Since(latestDate).Hours() / 24) + latestDay := utcDay(points[len(points)-1].Date) + ageDays := int(today.Sub(latestDay).Hours() / 24) if ageDays > maxSeriesAgeDays { return fmt.Errorf( "on-demand series is stale: most recent data point is %s (%d days old, maximum %d); check that the CE data feed is active or that the lookback date range ends near today", - latestDate.Format("2006-01-02"), ageDays, maxSeriesAgeDays, + latestDay.Format("2006-01-02"), ageDays, maxSeriesAgeDays, ) } return nil diff --git a/providers/aws/ladder/ladder_test.go b/providers/aws/ladder/ladder_test.go index 82a3b4c1d..7f971f0ec 100644 --- a/providers/aws/ladder/ladder_test.go +++ b/providers/aws/ladder/ladder_test.go @@ -816,7 +816,8 @@ func TestGetUsageBaseline_WrongScope_ReturnsError(t *testing.T) { func TestGetUsageBaseline_StaleSeries_ReturnsError(t *testing.T) { // Build a 30-point series whose most-recent point is maxSeriesAgeDays+1 days // in the past, simulating a CE feed that stopped (or a wrong date range). - staleEnd := time.Now().AddDate(0, 0, -(maxSeriesAgeDays + 1)).UTC().Truncate(24 * time.Hour) + // The date is built from the UTC calendar day so it is hour-independent. + staleEnd := utcDay(time.Now()).AddDate(0, 0, -(maxSeriesAgeDays + 1)) points := make([]DailyPoint, 30) for i := range points { points[i] = DailyPoint{ @@ -836,23 +837,51 @@ func TestGetUsageBaseline_StaleSeries_ReturnsError(t *testing.T) { // TestGetUsageBaseline_SparseSeries_ReturnsError is a regression test for gap G6a: // before coverage detection was added, a series with too many missing days for the // requested lookback window would silently yield a percentile over incomplete data. -// Now GetUsageBaseline must return an error when the series has fewer than -// lookbackDays - maxMissingDays points. +// Now GetUsageBaseline must return an error when fewer than +// lookbackDays - maxMissingDays points fall inside the window. func TestGetUsageBaseline_SparseSeries_ReturnsError(t *testing.T) { const lookbackDays = 30 // n is one point below the minimum: lookbackDays - maxMissingDays - 1. - // At n=26 (>= minBaselineSeriesDays=7) the coverage check fires, not the - // absolute-minimum check. + // All n points are recent (in-window), so at n=26 (>= minBaselineSeriesDays=7) + // the in-window coverage check fires, not the absolute-minimum check. n := lookbackDays - maxMissingDays - 1 cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(n)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) require.Error(t, err, "a series with too many gaps for the lookback window must be rejected") - assert.Contains(t, err.Error(), fmt.Sprintf("%d points", n), "error must report the actual point count") + assert.Contains(t, err.Error(), fmt.Sprintf("%d points", n), "error must report the actual in-window point count") assert.Contains(t, err.Error(), fmt.Sprintf("%d-day lookback", lookbackDays), "error must report the requested lookback") } +// TestGetUsageBaseline_InsufficientInWindowCoverage_ReturnsError is a regression +// test for the count-vs-window bug (CodeRabbit finding 1): the coverage check +// must count only points INSIDE the lookback window, not the total. Here there +// are 27 total points (enough to pass a naive total-count check) but only 20 +// fall within [today-30, today]; the oldest 7 are pushed far into the past. +func TestGetUsageBaseline_InsufficientInWindowCoverage_ReturnsError(t *testing.T) { + const lookbackDays = 30 + today := utcDay(time.Now()) + points := make([]DailyPoint, 0, 27) + // 7 points far outside the window (days -60..-54): defeat a naive count check. + for d := 60; d >= 54; d-- { + points = append(points, DailyPoint{Date: today.AddDate(0, 0, -d), USDPerHour: 1.0}) + } + // 20 fresh, in-window points (days -20..-1). + for d := 20; d >= 1; d-- { + points = append(points, DailyPoint{Date: today.AddDate(0, 0, -d), USDPerHour: 2.0}) + } + require.Len(t, points, 27, "27 total points, enough to defeat a total-count check") + + cov := &fakeCoverageSource{onDemandPoints: points} + a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) + + _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) + require.Error(t, err, "27 total but only 20 in-window points must be rejected") + assert.Contains(t, err.Error(), "20 points inside the 30-day lookback window", + "error must report the in-window count, not the total") +} + // TestGetUsageBaseline_FreshAndCompleteSeries_NoError is a regression test // confirming that the stale-series guard does not break the happy path: // a fresh, sufficiently complete series must still yield a valid baseline. @@ -871,10 +900,11 @@ func TestGetUsageBaseline_FreshAndCompleteSeries_NoError(t *testing.T) { // freshness comparison: a series whose most-recent point is EXACTLY // maxSeriesAgeDays old must pass (the check is age > max, not age >= max). func TestGetUsageBaseline_AgeExactlyAtBoundary_NoError(t *testing.T) { - // End the series exactly maxSeriesAgeDays days ago. Truncate to the UTC day - // and step back one extra hour so integer-day age floors to exactly - // maxSeriesAgeDays rather than maxSeriesAgeDays-1 due to sub-day rounding. - end := time.Now().Add(-time.Duration(maxSeriesAgeDays)*24*time.Hour - time.Hour).UTC().Truncate(24 * time.Hour) + // Most-recent point exactly maxSeriesAgeDays UTC calendar days ago -> age == + // maxSeriesAgeDays, which must pass (rejection is age > max). The date is + // built directly from the UTC calendar day (no hour arithmetic) so the test + // does not depend on the current UTC hour. + end := utcDay(time.Now()).AddDate(0, 0, -maxSeriesAgeDays) points := make([]DailyPoint, 30) for i := range points { points[i] = DailyPoint{Date: end.AddDate(0, 0, -(29 - i)), USDPerHour: float64(i + 1)} @@ -887,16 +917,16 @@ func TestGetUsageBaseline_AgeExactlyAtBoundary_NoError(t *testing.T) { } // TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError locks the off-by-one on -// the coverage comparison: exactly lookbackDays - maxMissingDays points must -// pass (the check is len < minRequired, not len <= minRequired). +// the coverage comparison: exactly lookbackDays - maxMissingDays in-window +// points must pass (the check is inWindow < minRequired, not <= minRequired). func TestGetUsageBaseline_CoverageExactlyAtBoundary_NoError(t *testing.T) { const lookbackDays = 30 - n := lookbackDays - maxMissingDays // exactly the minimum required + n := lookbackDays - maxMissingDays // exactly the minimum required, all in-window cov := &fakeCoverageSource{onDemandPoints: makeRecentPoints(n)} a := newTestLadder(t, &fakeRILister{}, &fakeSPLister{}, cov, &fakeUtilizationSource{}) _, err := a.GetUsageBaseline(context.Background(), testScope(), lookbackDays, 5.0) - require.NoError(t, err, "exactly lookbackDays-maxMissingDays points must pass (boundary is len < minRequired)") + require.NoError(t, err, "exactly lookbackDays-maxMissingDays in-window points must pass (boundary is inWindow < minRequired)") } // TestGetUsageBaseline_NonMonotonicDates_ReturnsError covers the chronology @@ -945,7 +975,9 @@ func makeRange(n int) []float64 { // keeps the series well within the maxSeriesAgeDays freshness window while // being realistic for a real CE feed. Ordered oldest-to-newest. func makeRecentPoints(n int) []DailyPoint { - yesterday := time.Now().AddDate(0, 0, -1).UTC().Truncate(24 * time.Hour) + // utcDay(now)-1 day = yesterday's UTC calendar day, constructed via calendar + // arithmetic so the series is independent of the current UTC hour. + yesterday := utcDay(time.Now()).AddDate(0, 0, -1) out := make([]DailyPoint, n) for i := range out { out[i] = DailyPoint{ @@ -959,7 +991,7 @@ func makeRecentPoints(n int) []DailyPoint { // makeConstantPoints returns n DailyPoints all with the given USDPerHour value, // ending yesterday. Ordered oldest-to-newest. func makeConstantPoints(n int, value float64) []DailyPoint { - yesterday := time.Now().AddDate(0, 0, -1).UTC().Truncate(24 * time.Hour) + yesterday := utcDay(time.Now()).AddDate(0, 0, -1) out := make([]DailyPoint, n) for i := range out { out[i] = DailyPoint{ From 4aff31f8291e78f16c59c773f1dd9ad42418ff8e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 16 Jul 2026 16:01:25 +0300 Subject: [PATCH 4/4] fix(aws): align factory noop series source with DailyPoint signature PR #1362 merged to main with factory.go's noopOnDemandSeriesSource returning []float64, while this branch changed the onDemandSeriesSource interface to return []DailyPoint (Date-carrying points for the stale-series freshness check). Merging without this alignment would break main despite no textual conflict (semantic conflict). Update the noop's return type to []DailyPoint. Behavior is unchanged: it still fails loud with the same not-yet-wired error, so plan runs against the unwired factory continue to be counted Errored. --- providers/aws/ladder/factory.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/aws/ladder/factory.go b/providers/aws/ladder/factory.go index 7a1df3c9e..9626cdcd3 100644 --- a/providers/aws/ladder/factory.go +++ b/providers/aws/ladder/factory.go @@ -88,7 +88,7 @@ func (noopRICoverageSource) GetRICoverageMap(_ context.Context, _ int, _ []strin // Real wiring (Cost Explorer adapter) arrives in PR-4. type noopOnDemandSeriesSource struct{} -func (noopOnDemandSeriesSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]float64, error) { +func (noopOnDemandSeriesSource) GetOnDemandSeries(_ context.Context, _ string, _ int) ([]DailyPoint, error) { return nil, fmt.Errorf("on-demand series source not yet wired: the Cost Explorer adapter is connected in PR-4; until then plan runs will be recorded as failed") }