make accumulate be able to govern windows (#3758)

Signed-off-by: Alex Meijer <ameijer@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

core/pkg/opencost/allocation.go

@@ -3301,6 +3301,8 @@ func (asr *AllocationSetRange) Accumulate(accumulateBy AccumulateOption) (*Alloc
 		return asr.accumulateByWeek()
 	case AccumulateOptionMonth:
 		return asr.accumulateByMonth()
+	case AccumulateOptionQuarter:
+		return asr.accumulateByQuarter()
 	default:
 		// ideally, this should never happen
 		return nil, fmt.Errorf("unexpected error, invalid accumulateByType: %s", accumulateBy)
@@ -3406,6 +3408,44 @@ func (asr *AllocationSetRange) accumulateByMonth() (*AllocationSetRange, error)
 	return result, nil
 }
 
+func (asr *AllocationSetRange) accumulateByQuarter() (*AllocationSetRange, error) {
+	var toAccumulate *AllocationSetRange
+	result := NewAllocationSetRange()
+	for i, as := range asr.Allocations {
+		if as.Window.Duration() != time.Hour*24 {
+			return nil, fmt.Errorf("window duration must equal 24 hours; got:%s", as.Window.Duration())
+		}
+
+		year, month, _ := as.Window.Start().Date()
+		nextDay := as.Window.Start().Add(time.Hour * 24)
+		nextYear, nextMonth, _ := nextDay.Date()
+		quarter := (int(month) - 1) / 3
+		nextQuarter := (int(nextMonth) - 1) / 3
+
+		if toAccumulate == nil {
+			toAccumulate = NewAllocationSetRange()
+			as = as.Clone()
+		}
+
+		toAccumulate.Append(as)
+		asAccumulated, err := toAccumulate.accumulate()
+		if err != nil {
+			return nil, fmt.Errorf("error accumulating result: %s", err)
+		}
+		toAccumulate = NewAllocationSetRange(asAccumulated)
+
+		// either the quarter has ended, or there are no more allocation sets
+		if quarter != nextQuarter || year != nextYear || i == len(asr.Allocations)-1 {
+			if length := len(toAccumulate.Allocations); length != 1 {
+				return nil, fmt.Errorf("failed accumulation, detected %d sets instead of 1", length)
+			}
+			result.Append(toAccumulate.Allocations[0])
+			toAccumulate = nil
+		}
+	}
+	return result, nil
+}
+
 func (asr *AllocationSetRange) accumulateByWeek() (*AllocationSetRange, error) {
 	if len(asr.Allocations) > 0 && asr.Allocations[0].Window.Duration() == timeutil.Week {
 		return asr, nil

core/pkg/opencost/allocation_test.go

@@ -2779,6 +2779,36 @@ func TestAllocationSetRange_AccumulateBy_Month(t *testing.T) {
 	}
 }
 
+func TestAllocationSetRange_AccumulateBy_Quarter(t *testing.T) {
+	q1Day1 := time.Date(2020, 3, 30, 0, 0, 0, 0, time.UTC)
+	q1Day2 := time.Date(2020, 3, 31, 0, 0, 0, 0, time.UTC)
+	q2Day1 := time.Date(2020, 4, 1, 0, 0, 0, 0, time.UTC)
+	q2Day2 := time.Date(2020, 4, 2, 0, 0, 0, 0, time.UTC)
+
+	q1AS1 := NewAllocationSet(q1Day1, q1Day2)
+	q1AS1.Set(NewMockUnitAllocation("", q1Day1, day, nil))
+	q1AS2 := NewAllocationSet(q1Day2, q2Day1)
+	q1AS2.Set(NewMockUnitAllocation("", q1Day2, day, nil))
+	q2AS1 := NewAllocationSet(q2Day1, q2Day2)
+	q2AS1.Set(NewMockUnitAllocation("", q2Day1, day, nil))
+
+	asr := NewAllocationSetRange(q1AS1, q1AS2, q2AS1)
+	asr, err := asr.Accumulate(AccumulateOptionQuarter)
+	if err != nil {
+		t.Fatalf("unexpected error calling accumulateBy quarter: %s", err)
+	}
+
+	if len(asr.Allocations) != 2 {
+		t.Fatalf("expected 2 allocation sets, got:%d", len(asr.Allocations))
+	}
+
+	for _, as := range asr.Allocations {
+		if as.Window.Duration() < time.Hour*24 || as.Window.Duration() > time.Hour*24*92 {
+			t.Fatalf("expected window duration to be between 1 and 92 days, got:%s", as.Window.Duration().String())
+		}
+	}
+}
+
 // TODO niko
 // func TestAllocationSetRange_AggregateBy(t *testing.T) {}
 

pkg/costmodel/aggregation.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"net/http"
 	"strings"
+	"time"
 
 	"github.com/julienschmidt/httprouter"
 
@@ -57,6 +58,149 @@ func ParseAggregationProperties(aggregations []string) ([]string, error) {
 	return aggregateBy, nil
 }
 
+func resolveAccumulateOption(accumulate opencost.AccumulateOption, accumulateBy string) (opencost.AccumulateOption, error) {
+	accumulateByRaw := strings.TrimSpace(strings.ToLower(accumulateBy))
+	if accumulateByRaw == "" {
+		return accumulate, nil
+	}
+
+	if accumulateByRaw == "all" {
+		return opencost.AccumulateOptionAll, nil
+	}
+
+	if accumulateByRaw == "none" {
+		return opencost.AccumulateOptionNone, nil
+	}
+
+	accumulateByOpt := opencost.ParseAccumulate(accumulateByRaw)
+	if accumulateByOpt == opencost.AccumulateOptionNone {
+		return opencost.AccumulateOptionNone, fmt.Errorf("invalid accumulateBy option: %s", accumulateBy)
+	}
+
+	return accumulateByOpt, nil
+}
+
+func resolveAccumulateFromQuery(qp httputil.QueryParams) opencost.AccumulateOption {
+	rawAccumulate := strings.TrimSpace(qp.Get("accumulate", ""))
+	if strings.EqualFold(rawAccumulate, string(opencost.AccumulateOptionAll)) {
+		return opencost.AccumulateOptionAll
+	}
+
+	accumulate := opencost.ParseAccumulate(rawAccumulate)
+	if accumulate == opencost.AccumulateOptionNone && qp.GetBool("accumulate", false) {
+		return opencost.AccumulateOptionAll
+	}
+
+	return accumulate
+}
+
+func resolveStepForAccumulate(step time.Duration, accumulateBy opencost.AccumulateOption) time.Duration {
+	const (
+		day  = 24 * time.Hour
+		week = 7 * day
+	)
+
+	switch accumulateBy {
+	case opencost.AccumulateOptionHour:
+		return time.Hour
+	case opencost.AccumulateOptionDay:
+		// day accumulation supports either hourly or already-daily sets
+		if step == day {
+			return day
+		}
+		return time.Hour
+	case opencost.AccumulateOptionWeek, opencost.AccumulateOptionMonth, opencost.AccumulateOptionQuarter:
+		// week accumulation supports either daily or already-weekly sets
+		if accumulateBy == opencost.AccumulateOptionWeek && step == week {
+			return week
+		}
+		return day
+	default:
+		return step
+	}
+}
+
+func resolveDefaultStepFromAccumulate(window opencost.Window, accumulateBy opencost.AccumulateOption) time.Duration {
+	switch accumulateBy {
+	case opencost.AccumulateOptionHour:
+		return time.Hour
+	case opencost.AccumulateOptionDay:
+		return 24 * time.Hour
+	case opencost.AccumulateOptionWeek:
+		return 7 * 24 * time.Hour
+	case opencost.AccumulateOptionMonth, opencost.AccumulateOptionQuarter:
+		// month/quarter accumulation requires daily input sets
+		return 24 * time.Hour
+	case opencost.AccumulateOptionAll:
+		return window.Duration()
+	default:
+		return window.Duration()
+	}
+}
+
+func resolveStepFromQuery(qp httputil.QueryParams, window opencost.Window, accumulateBy opencost.AccumulateOption) (time.Duration, error) {
+	stepRaw := strings.TrimSpace(strings.ToLower(qp.Get("step", "")))
+	if stepRaw == "" {
+		step := resolveDefaultStepFromAccumulate(window, accumulateBy)
+		return resolveStepForAccumulate(step, accumulateBy), nil
+	}
+
+	switch stepRaw {
+	case "hour":
+		return resolveStepForAccumulate(time.Hour, accumulateBy), nil
+	case "day":
+		return resolveStepForAccumulate(24*time.Hour, accumulateBy), nil
+	case "week":
+		return resolveStepForAccumulate(7*24*time.Hour, accumulateBy), nil
+	case "month":
+		// month accumulation operates on daily inputs and calendar-rounded query windows
+		return resolveStepForAccumulate(24*time.Hour, accumulateBy), nil
+	case "quarter":
+		// quarter accumulation operates on daily inputs and calendar-rounded query windows
+		return resolveStepForAccumulate(24*time.Hour, accumulateBy), nil
+	default:
+		step, err := time.ParseDuration(stepRaw)
+		if err != nil {
+			return 0, fmt.Errorf("invalid step %q: must be a Go duration or one of hour, day, week, month, quarter: %w", stepRaw, err)
+		}
+		return resolveStepForAccumulate(step, accumulateBy), nil
+	}
+}
+
+func resolveQueryWindowForAccumulate(window opencost.Window, accumulateBy opencost.AccumulateOption) (opencost.Window, error) {
+	switch accumulateBy {
+	case opencost.AccumulateOptionHour, opencost.AccumulateOptionDay, opencost.AccumulateOptionWeek, opencost.AccumulateOptionMonth, opencost.AccumulateOptionQuarter:
+		windows, err := window.GetAccumulateWindows(accumulateBy)
+		if err != nil {
+			return opencost.Window{}, err
+		}
+		if len(windows) == 0 {
+			return opencost.Window{}, fmt.Errorf("no query windows for accumulate option %s", accumulateBy)
+		}
+
+		return opencost.NewClosedWindow(*windows[0].Start(), *windows[len(windows)-1].End()), nil
+	default:
+		return window, nil
+	}
+}
+
+func trimAllocationSetRangeToRequestWindow(asr *opencost.AllocationSetRange, requestWindow opencost.Window) *opencost.AllocationSetRange {
+	if asr == nil {
+		return nil
+	}
+
+	trimmed := opencost.NewAllocationSetRange()
+	trimmed.FromStore = asr.FromStore
+	for _, as := range asr.Allocations {
+		// Keep only sets that overlap the originally requested window.
+		if as.Start().Before(*requestWindow.End()) && as.End().After(*requestWindow.Start()) {
+			trimmed.Append(as)
+		}
+	}
+
+	return trimmed
+}
+
 func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *http.Request, ps httprouter.Params) {
 	w.Header().Set("Content-Type", "application/json")
 
@@ -72,8 +216,6 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 	// Step is an optional parameter that defines the duration per-set, i.e.
 	// the window for an AllocationSet, of the AllocationSetRange to be
 	// computed. Defaults to the window size, making one set.
-	step := qp.GetDuration("step", window.Duration())
-
 	// Aggregation is a required comma-separated list of fields by which to
 	// aggregate results. Some fields allow a sub-field, which is distinguished
 	// with a colon; e.g. "label:app".
@@ -84,9 +226,24 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 		http.Error(w, fmt.Sprintf("Invalid 'aggregate' parameter: %s", err), http.StatusBadRequest)
 	}
 
-	// Accumulate is an optional parameter, defaulting to false, which if true
-	// sums each Set in the Range, producing one Set.
-	accumulate := qp.GetBool("accumulate", false)
+	// Accumulate is an optional parameter that accepts bool-style values (e.g.
+	// true/1) or options (e.g. day/week/month) and governs accumulation windowing.
+	accumulateOpt := resolveAccumulateFromQuery(qp)
+	accumulateBy, err := resolveAccumulateOption(accumulateOpt, qp.Get("accumulateBy", ""))
+	if err != nil {
+		proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid 'accumulateBy' parameter: %s", err)))
+		return
+	}
+	step, err := resolveStepFromQuery(qp, window, accumulateBy)
+	if err != nil {
+		proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid step parameter: %s", err)))
+		return
+	}
+	queryWindow, err := resolveQueryWindowForAccumulate(window, accumulateBy)
+	if err != nil {
+		proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid accumulation configuration: %s", err)))
+		return
+	}
 
 	// Get allocation filter if provided
 	allocationFilter := qp.Get("filter", "")
@@ -94,8 +251,8 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 	// Query for AllocationSets in increments of the given step duration,
 	// appending each to the AllocationSetRange.
 	asr := opencost.NewAllocationSetRange()
-	stepStart := *window.Start()
-	for window.End().After(stepStart) {
+	stepStart := *queryWindow.Start()
+	for queryWindow.End().After(stepStart) {
 		stepEnd := stepStart.Add(step)
 		stepWindow := opencost.NewWindow(&stepStart, &stepEnd)
 
@@ -124,7 +281,7 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 			return
 		}
 		filteredASR := opencost.NewAllocationSetRange()
-		for _, as := range asr.Slice() {
+		for _, as := range asr.Allocations {
 			filteredAS := opencost.NewAllocationSet(as.Start(), as.End())
 			for _, alloc := range as.Allocations {
 				if matcher.Matches(alloc) {
@@ -148,16 +305,18 @@ func (a *Accesses) ComputeAllocationHandlerSummary(w http.ResponseWriter, r *htt
 	}
 
 	// Accumulate, if requested
-	if accumulate {
-		asr, err = asr.Accumulate(opencost.AccumulateOptionAll)
+	if accumulateBy != opencost.AccumulateOptionNone {
+		asr, err = asr.Accumulate(accumulateBy)
 		if err != nil {
 			proto.WriteError(w, proto.InternalServerError(err.Error()))
 			return
 		}
+
+		asr = trimAllocationSetRangeToRequestWindow(asr, window)
 	}
 
 	sasl := []*opencost.SummaryAllocationSet{}
-	for _, as := range asr.Slice() {
+	for _, as := range asr.Allocations {
 		sas := opencost.NewSummaryAllocationSet(as, nil, nil, false, false)
 		sasl = append(sasl, sas)
 	}
@@ -182,8 +341,6 @@ func (a *Accesses) ComputeAllocationHandler(w http.ResponseWriter, r *http.Reque
 	// Step is an optional parameter that defines the duration per-set, i.e.
 	// the window for an AllocationSet, of the AllocationSetRange to be
 	// computed. Defaults to the window size, making one set.
-	step := qp.GetDuration("step", window.Duration())
-
 	// Aggregation is an optional comma-separated list of fields by which to
 	// aggregate results. Some fields allow a sub-field, which is distinguished
 	// with a colon; e.g. "label:app".
@@ -196,18 +353,21 @@ func (a *Accesses) ComputeAllocationHandler(w http.ResponseWriter, r *http.Reque
 
 	// IncludeIdle, if true, uses Asset data to incorporate Idle Allocation
 	includeIdle := qp.GetBool("includeIdle", false)
-	// Accumulate is an optional parameter, defaulting to false, which if true
-	// sums each Set in the Range, producing one Set.
-	accumulate := qp.GetBool("accumulate", false)
-
-	// Accumulate is an optional parameter that accumulates an AllocationSetRange
-	// by the resolution of the given time duration.
-	// Defaults to 0. If a value is not passed then the parameter is not used.
-	accumulateBy := opencost.AccumulateOption(qp.Get("accumulateBy", ""))
-
-	// if accumulateBy is not explicitly set, and accumulate is true, ensure result is accumulated
-	if accumulateBy == opencost.AccumulateOptionNone && accumulate {
-		accumulateBy = opencost.AccumulateOptionAll
+	// Accumulate is an optional parameter that accepts bool-style values (e.g.
+	// true/1) or options (e.g. day/week/month) and governs accumulation windowing.
+	accumulateOpt := resolveAccumulateFromQuery(qp)
+
+	// AccumulateBy is an optional parameter that overrides accumulate with an
+	// explicit accumulation option (e.g. all/day/week/month/quarter/none).
+	accumulateBy, err := resolveAccumulateOption(accumulateOpt, qp.Get("accumulateBy", ""))
+	if err != nil {
+		proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid 'accumulateBy' parameter: %s", err)))
+		return
+	}
+	step, err := resolveStepFromQuery(qp, window, accumulateBy)
+	if err != nil {
+		proto.WriteError(w, proto.BadRequest(fmt.Sprintf("Invalid step parameter: %s", err)))
+		return
 	}
 
 	// IdleByNode, if true, computes idle allocations at the node level.

pkg/costmodel/aggregation_test.go

@@ -1,9 +1,12 @@
 package costmodel
 
 import (
+	"net/url"
 	"testing"
+	"time"
 
 	"github.com/opencost/opencost/core/pkg/opencost"
+	"github.com/opencost/opencost/core/pkg/util/httputil"
 )
 
 func TestParseAggregationProperties_Default(t *testing.T) {
@@ -42,3 +45,391 @@ func TestParseAggregationProperties_All(t *testing.T) {
 		t.Fatalf("TestParseAggregationPropertiesDefault: expected length of 0, got: %d", len(got))
 	}
 }
+
+func TestResolveAccumulateOption(t *testing.T) {
+	tests := []struct {
+		name       string
+		accumulate opencost.AccumulateOption
+		input      string
+		expected   opencost.AccumulateOption
+		expectErr  bool
+	}{
+		{
+			name:       "accumulate false without accumulateBy",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      "",
+			expected:   opencost.AccumulateOptionNone,
+		},
+		{
+			name:       "accumulate true without accumulateBy defaults to all",
+			accumulate: opencost.AccumulateOptionAll,
+			input:      "",
+			expected:   opencost.AccumulateOptionAll,
+		},
+		{
+			name:       "accumulate day is preserved",
+			accumulate: opencost.AccumulateOptionDay,
+			input:      "",
+			expected:   opencost.AccumulateOptionDay,
+		},
+		{
+			name:       "accumulate week is preserved",
+			accumulate: opencost.AccumulateOptionWeek,
+			input:      "",
+			expected:   opencost.AccumulateOptionWeek,
+		},
+		{
+			name:       "accumulateBy overrides accumulate",
+			accumulate: opencost.AccumulateOptionDay,
+			input:      string(opencost.AccumulateOptionWeek),
+			expected:   opencost.AccumulateOptionWeek,
+		},
+		{
+			name:       "accumulate none with explicit accumulateBy",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      string(opencost.AccumulateOptionHour),
+			expected:   opencost.AccumulateOptionHour,
+		},
+		{
+			name:       "accumulateBy none is valid",
+			accumulate: opencost.AccumulateOptionWeek,
+			input:      "none",
+			expected:   opencost.AccumulateOptionNone,
+		},
+		{
+			name:       "accumulateBy all is valid",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      "all",
+			expected:   opencost.AccumulateOptionAll,
+		},
+		{
+			name:       "accumulateBy normalizes case",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      "Week",
+			expected:   opencost.AccumulateOptionWeek,
+		},
+		{
+			name:       "accumulateBy quarter is valid",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      string(opencost.AccumulateOptionQuarter),
+			expected:   opencost.AccumulateOptionQuarter,
+		},
+		{
+			name:       "accumulate quarter is preserved",
+			accumulate: opencost.AccumulateOptionQuarter,
+			input:      "",
+			expected:   opencost.AccumulateOptionQuarter,
+		},
+		{
+			name:       "invalid accumulateBy is flagged",
+			accumulate: opencost.AccumulateOptionNone,
+			input:      "nonsense",
+			expected:   opencost.AccumulateOptionNone,
+			expectErr:  true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := resolveAccumulateOption(tc.accumulate, tc.input)
+			if tc.expectErr && err == nil {
+				t.Fatalf("expected error but got nil")
+			}
+			if !tc.expectErr && err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+			if got != tc.expected {
+				t.Fatalf("expected %q, got %q", tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestResolveAccumulateFromQuery_BackwardCompatibleTruthyValues(t *testing.T) {
+	tests := []struct {
+		name  string
+		input string
+	}{
+		{name: "true supported", input: "true"},
+		{name: "all supported", input: "all"},
+		{name: "1 supported", input: "1"},
+		{name: "t supported", input: "t"},
+		{name: "TRUE supported", input: "TRUE"},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			values := url.Values{}
+			values.Set("accumulate", tc.input)
+			qp := httputil.NewQueryParams(values)
+			got := resolveAccumulateFromQuery(qp)
+			if got != opencost.AccumulateOptionAll {
+				t.Fatalf("expected %q for %q, got %q", opencost.AccumulateOptionAll, tc.input, got)
+			}
+		})
+	}
+}
+
+func TestResolveStepForAccumulate(t *testing.T) {
+	tests := []struct {
+		name         string
+		step         time.Duration
+		accumulateBy opencost.AccumulateOption
+		expected     time.Duration
+	}{
+		{
+			name:         "none keeps requested step",
+			step:         14 * 24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionNone,
+			expected:     14 * 24 * time.Hour,
+		},
+		{
+			name:         "day uses hourly step",
+			step:         14 * 24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionDay,
+			expected:     time.Hour,
+		},
+		{
+			name:         "day keeps daily step",
+			step:         24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionDay,
+			expected:     24 * time.Hour,
+		},
+		{
+			name:         "week uses daily step",
+			step:         14 * 24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionWeek,
+			expected:     24 * time.Hour,
+		},
+		{
+			name:         "week keeps weekly step",
+			step:         7 * 24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionWeek,
+			expected:     7 * 24 * time.Hour,
+		},
+		{
+			name:         "quarter uses daily step",
+			step:         7 * 24 * time.Hour,
+			accumulateBy: opencost.AccumulateOptionQuarter,
+			expected:     24 * time.Hour,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := resolveStepForAccumulate(tc.step, tc.accumulateBy)
+			if got != tc.expected {
+				t.Fatalf("expected %v, got %v", tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestResolveDefaultStepFromAccumulate(t *testing.T) {
+	window := opencost.NewClosedWindow(
+		time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 4, 15, 0, 0, 0, 0, time.UTC),
+	)
+
+	tests := []struct {
+		name         string
+		accumulateBy opencost.AccumulateOption
+		expected     time.Duration
+	}{
+		{
+			name:         "none defaults to window duration",
+			accumulateBy: opencost.AccumulateOptionNone,
+			expected:     window.Duration(),
+		},
+		{
+			name:         "day defaults to daily",
+			accumulateBy: opencost.AccumulateOptionDay,
+			expected:     24 * time.Hour,
+		},
+		{
+			name:         "week defaults to weekly",
+			accumulateBy: opencost.AccumulateOptionWeek,
+			expected:     7 * 24 * time.Hour,
+		},
+		{
+			name:         "month defaults to daily",
+			accumulateBy: opencost.AccumulateOptionMonth,
+			expected:     24 * time.Hour,
+		},
+		{
+			name:         "all defaults to window duration",
+			accumulateBy: opencost.AccumulateOptionAll,
+			expected:     window.Duration(),
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := resolveDefaultStepFromAccumulate(window, tc.accumulateBy)
+			if got != tc.expected {
+				t.Fatalf("expected %v, got %v", tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestResolveStepFromQuery(t *testing.T) {
+	window := opencost.NewClosedWindow(
+		time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 4, 15, 0, 0, 0, 0, time.UTC),
+	)
+
+	tests := []struct {
+		name         string
+		stepRaw      string
+		accumulateBy opencost.AccumulateOption
+		expected     time.Duration
+		expectErr    bool
+	}{
+		{
+			name:         "unset step defaults from weekly accumulate",
+			stepRaw:      "",
+			accumulateBy: opencost.AccumulateOptionWeek,
+			expected:     7 * 24 * time.Hour,
+		},
+		{
+			name:         "monthly step keyword supported",
+			stepRaw:      "month",
+			accumulateBy: opencost.AccumulateOptionNone,
+			expected:     24 * time.Hour,
+		},
+		{
+			name:         "weekly step keyword supported",
+			stepRaw:      "week",
+			accumulateBy: opencost.AccumulateOptionWeek,
+			expected:     7 * 24 * time.Hour,
+		},
+		{
+			name:         "invalid duration errors",
+			stepRaw:      "not-a-duration",
+			accumulateBy: opencost.AccumulateOptionNone,
+			expectErr:    true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			values := url.Values{}
+			if tc.stepRaw != "" {
+				values.Set("step", tc.stepRaw)
+			}
+			qp := httputil.NewQueryParams(values)
+			got, err := resolveStepFromQuery(qp, window, tc.accumulateBy)
+			if tc.expectErr && err == nil {
+				t.Fatalf("expected error but got nil")
+			}
+			if tc.expectErr {
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %s", err)
+			}
+			if got != tc.expected {
+				t.Fatalf("expected %v, got %v", tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestWeeklyAccumulateTwoWeeksProducesTwoSets(t *testing.T) {
+	start := time.Date(2026, 4, 5, 0, 0, 0, 0, time.UTC) // Sunday
+	end := start.Add(14 * 24 * time.Hour)
+	requestedStep := end.Sub(start)
+
+	accumulateBy, err := resolveAccumulateOption(opencost.AccumulateOptionNone, string(opencost.AccumulateOptionWeek))
+	if err != nil {
+		t.Fatalf("unexpected error resolving accumulate option: %s", err)
+	}
+	step := resolveStepForAccumulate(requestedStep, accumulateBy)
+	if step != 24*time.Hour {
+		t.Fatalf("expected daily step for weekly accumulation, got %v", step)
+	}
+
+	asr := opencost.NewAllocationSetRange()
+	for ts := start; ts.Before(end); ts = ts.Add(step) {
+		next := ts.Add(step)
+		as := opencost.NewAllocationSet(ts, next)
+		as.Set(opencost.NewMockUnitAllocation("workload", ts, step, nil))
+		asr.Append(as)
+	}
+
+	weekly, err := asr.Accumulate(opencost.AccumulateOptionWeek)
+	if err != nil {
+		t.Fatalf("unexpected weekly accumulate error: %s", err)
+	}
+
+	if len(weekly.Allocations) != 2 {
+		t.Fatalf("expected 2 weekly sets from 2 weeks of data, got %d", len(weekly.Allocations))
+	}
+
+	for i, as := range weekly.Allocations {
+		if got := as.Window.Duration(); got != 7*24*time.Hour {
+			t.Fatalf("set %d expected 7d window, got %s", i, got)
+		}
+	}
+}
+
+func TestResolveQueryWindowForAccumulate_WeekRoundsToCalendarWeeks(t *testing.T) {
+	start := time.Date(2026, 4, 6, 0, 0, 0, 0, time.UTC) // Monday
+	end := start.Add(14 * 24 * time.Hour)
+	window := opencost.NewClosedWindow(start, end)
+
+	got, err := resolveQueryWindowForAccumulate(window, opencost.AccumulateOptionWeek)
+	if err != nil {
+		t.Fatalf("unexpected error: %s", err)
+	}
+
+	expectedStart := time.Date(2026, 4, 5, 0, 0, 0, 0, time.UTC) // Sunday
+	expectedEnd := time.Date(2026, 4, 26, 0, 0, 0, 0, time.UTC)  // Sunday after 3 calendar weeks
+	if !got.Start().Equal(expectedStart) {
+		t.Fatalf("expected rounded start %s, got %s", expectedStart, got.Start())
+	}
+	if !got.End().Equal(expectedEnd) {
+		t.Fatalf("expected rounded end %s, got %s", expectedEnd, got.End())
+	}
+}
+
+func TestTrimAllocationSetRangeToRequestWindow(t *testing.T) {
+	requestStart := time.Date(2026, 4, 13, 0, 0, 0, 0, time.UTC)
+	requestEnd := time.Date(2026, 4, 26, 0, 0, 0, 0, time.UTC)
+	requestWindow := opencost.NewClosedWindow(requestStart, requestEnd)
+
+	before := opencost.NewAllocationSet(
+		time.Date(2026, 4, 5, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 4, 12, 0, 0, 0, 0, time.UTC),
+	)
+	overlap := opencost.NewAllocationSet(
+		time.Date(2026, 4, 12, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 4, 19, 0, 0, 0, 0, time.UTC),
+	)
+	inside := opencost.NewAllocationSet(
+		time.Date(2026, 4, 19, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 4, 26, 0, 0, 0, 0, time.UTC),
+	)
+	after := opencost.NewAllocationSet(
+		time.Date(2026, 4, 26, 0, 0, 0, 0, time.UTC),
+		time.Date(2026, 5, 3, 0, 0, 0, 0, time.UTC),
+	)
+
+	asr := opencost.NewAllocationSetRange(before, overlap, inside, after)
+	asr.FromStore = "test-store"
+	trimmed := trimAllocationSetRangeToRequestWindow(asr, requestWindow)
+
+	if len(trimmed.Allocations) != 2 {
+		t.Fatalf("expected 2 overlapping sets, got %d", len(trimmed.Allocations))
+	}
+	if !trimmed.Allocations[0].Start().Equal(overlap.Start()) {
+		t.Fatalf("expected first set to start at %s, got %s", overlap.Start(), trimmed.Allocations[0].Start())
+	}
+	if !trimmed.Allocations[1].Start().Equal(inside.Start()) {
+		t.Fatalf("expected second set to start at %s, got %s", inside.Start(), trimmed.Allocations[1].Start())
+	}
+	if trimmed.FromStore != asr.FromStore {
+		t.Fatalf("expected FromStore to be preserved")
+	}
+}

pkg/costmodel/costmodel.go

@@ -1603,13 +1603,17 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 
 	// Begin with empty response
 	asr := opencost.NewAllocationSetRange()
+	queryWindow, err := resolveQueryWindowForAccumulate(window, accumulateBy)
+	if err != nil {
+		return nil, fmt.Errorf("invalid accumulation configuration: %w", err)
+	}
 
 	// Query for AllocationSets in increments of the given step duration,
 	// appending each to the response.
-	stepStart := *window.Start()
+	stepStart := *queryWindow.Start()
 	stepEnd := stepStart.Add(step)
 	var isAKS bool
-	for window.End().After(stepStart) {
+	for queryWindow.End().After(stepStart) {
 		allocSet, err := cm.ComputeAllocation(stepStart, stepEnd)
 		if err != nil {
 			return nil, fmt.Errorf("error computing allocations for %s: %w", opencost.NewClosedWindow(stepStart, stepEnd), err)
@@ -1669,7 +1673,7 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 			return nil, fmt.Errorf("failed to compile filter: %w", err)
 		}
 		filteredASR := opencost.NewAllocationSetRange()
-		for _, as := range asr.Slice() {
+		for _, as := range asr.Allocations {
 			filteredAS := opencost.NewAllocationSet(as.Start(), as.End())
 			for _, alloc := range as.Allocations {
 				if matcher.Matches(alloc) {
@@ -1699,7 +1703,7 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 	}
 
 	// Aggregate
-	err := asr.AggregateBy(aggregate, opts)
+	err = asr.AggregateBy(aggregate, opts)
 	if err != nil {
 		return nil, fmt.Errorf("error aggregating for %s: %w", window, err)
 	}
@@ -1712,6 +1716,8 @@ func (cm *CostModel) QueryAllocation(window opencost.Window, step time.Duration,
 			return nil, fmt.Errorf("error accumulating by %v: %s", accumulateBy, err)
 		}
 
+		asr = trimAllocationSetRangeToRequestWindow(asr, window)
+
 		// when accumulating and returning PARCs, we need the totals for the
 		// accumulated windows to accurately compute a fraction
 		if includeProportionalAssetResourceCosts {