perf(api): page recommendations from an ID-ordered keyset scan instead of the full list

internal/api/handler_recommendations.go

@@ -106,6 +106,10 @@ func (h *Handler) getRecommendations(ctx context.Context, req *events.LambdaFunc
 		return nil, fmt.Errorf("failed to get allowed accounts: %w", err)
 	}
 
+	if page.enabled {
+		return h.getRecommendationsPage(ctx, scope, filter, page)
+	}
+
 	recommendations, err := h.scheduler.ListRecommendations(recommendationsReadContext(ctx, scope), filter)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get recommendations: %w", err)
@@ -126,7 +130,7 @@ func (h *Handler) getRecommendations(ctx context.Context, req *events.LambdaFunc
 		return nil, err
 	}
 
-	return buildRecommendationsPageResponse(recommendations, page)
+	return buildRecommendationsResponse(recommendations), nil
 }
 
 // filterRecommendationsByAllowedAccounts filters recommendations to only include
@@ -146,10 +150,18 @@ func (h *Handler) filterRecommendationsByScope(ctx context.Context, allowedAccou
 	if allowedAccounts.AllowsAll() {
 		return recs, nil
 	}
+	nameByID, err := h.cloudAccountNamesByID(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return filterRecommendationsInScope(allowedAccounts, nameByID, recs), nil
+}
 
-	// Resolve account names so the allowed list can match on either ID or
-	// display name. Recommendations only carry CloudAccountID, so the name
-	// lookup needs a one-time fetch of the account list.
+// cloudAccountNamesByID resolves account names so an allowed-accounts list
+// can match on either ID or display name. Recommendations only carry
+// CloudAccountID, so the name lookup needs a one-time fetch of the account
+// list.
+func (h *Handler) cloudAccountNamesByID(ctx context.Context) (map[string]string, error) {
 	accounts, err := h.config.ListCloudAccounts(ctx, config.CloudAccountFilter{})
 	if err != nil {
 		return nil, fmt.Errorf("failed to list accounts for filter: %w", err)
@@ -159,7 +171,12 @@ func (h *Handler) filterRecommendationsByScope(ctx context.Context, allowedAccou
 		a := accounts[_rvc]
 		nameByID[a.ID] = a.Name
 	}
+	return nameByID, nil
+}
 
+// filterRecommendationsInScope keeps the recs whose account allowedAccounts
+// allows, in place and in order. Recs without an account are dropped.
+func filterRecommendationsInScope(allowedAccounts auth.AccountScope, nameByID map[string]string, recs []config.RecommendationRecord) []config.RecommendationRecord {
 	filtered := recs[:0]
 	for _rvc := range recs {
 		rec := recs[_rvc]
@@ -171,7 +188,7 @@ func (h *Handler) filterRecommendationsByScope(ctx context.Context, allowedAccou
 			filtered = append(filtered, rec)
 		}
 	}
-	return filtered, nil
+	return filtered
 }
 
 // filterRecommendationsByEnabledProviders removes recommendations for providers
@@ -210,15 +227,9 @@ func buildRecommendationsResponse(recommendations []config.RecommendationRecord)
 // itself or one page of it (issue #723), so the totals never shrink to the
 // page.
 func summarizeRecommendations(all, rows []config.RecommendationRecord) *RecommendationsResponse {
-	regionSet := make(map[string]struct{})
-	var totalSavings, totalUpfront float64
+	var summary recommendationsSummary
 	for i := range all {
-		rec := &all[i]
-		totalSavings += rec.Savings
-		totalUpfront += rec.UpfrontCost
-		if rec.Region != "" {
-			regionSet[rec.Region] = struct{}{}
-		}
+		summary.add(&all[i])
 	}
 	for i := range rows {
 		// Surface the compute size (VCPU / MemoryGB) at the top level so
@@ -232,23 +243,51 @@ func summarizeRecommendations(all, rows []config.RecommendationRecord) *Recommen
 		// renders "—". Only returned rows are decoded.
 		stampComputeCapacity(&rows[i])
 	}
+	return summary.response(rows)
+}
+
+// recommendationsSummary accumulates the full-set totals and region list of
+// a list response one rec at a time, so a paginated scan can total every
+// filtered row without holding them all (issue #981).
+type recommendationsSummary struct {
+	count        int
+	totalSavings float64
+	totalUpfront float64
+	regionSet    map[string]struct{}
+}
+
+func (s *recommendationsSummary) add(rec *config.RecommendationRecord) {
+	s.count++
+	s.totalSavings += rec.Savings
+	s.totalUpfront += rec.UpfrontCost
+	if rec.Region == "" {
+		return
+	}
+	if s.regionSet == nil {
+		s.regionSet = make(map[string]struct{})
+	}
+	s.regionSet[rec.Region] = struct{}{}
+}
 
-	regions := make([]string, 0, len(regionSet))
-	for r := range regionSet {
+// response returns rows as the response body with the accumulated summary
+// and regions.
+func (s *recommendationsSummary) response(rows []config.RecommendationRecord) *RecommendationsResponse {
+	regions := make([]string, 0, len(s.regionSet))
+	for r := range s.regionSet {
 		regions = append(regions, r)
 	}
 
 	var avgPayback float64
-	if totalSavings > 0 {
-		avgPayback = totalUpfront / totalSavings
+	if s.totalSavings > 0 {
+		avgPayback = s.totalUpfront / s.totalSavings
 	}
 
 	return &RecommendationsResponse{
 		Recommendations: rows,
 		Summary: RecommendationsSummary{
-			TotalCount:          len(all),
-			TotalMonthlySavings: totalSavings,
-			TotalUpfrontCost:    totalUpfront,
+			TotalCount:          s.count,
+			TotalMonthlySavings: s.totalSavings,
+			TotalUpfrontCost:    s.totalUpfront,
 			AvgPaybackMonths:    avgPayback,
 		},
 		Regions: regions,

internal/api/handler_recommendations_page.go

@@ -4,7 +4,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -18,9 +17,8 @@ import (
 // which passes the 6 MB Lambda response cap once a tenant accumulates a few
 // thousand recommendations. A caller that sends `limit` (and, for later
 // pages, the `cursor` returned as `next_cursor`) gets one bounded page.
-// This bounds the response only: every request still reads, filters and
-// sorts the whole filtered set in memory and then slices the page from it
-// (store-side paging is tracked in #981). A
+// A paginated request scans the filtered cache in ID-ordered chunks and keeps
+// only the page (issue #981, see handler_recommendations_scan.go). A
 // page is bounded twice: by row count (limit) and by serialized bytes
 // (recommendationsPageByteBudget), because the opaque Details blob makes the
 // size of a row data-dependent.
@@ -120,34 +118,6 @@ func decodeRecommendationsCursor(cursor string) (string, error) {
 	return string(raw), nil
 }
 
-// sortRecommendationsAfter sorts recs by ID in place (it is this request's
-// own slice, freshly read from the store) and returns the index of the first
-// row strictly after afterID.
-//
-// The ID is the application recommendation ID that the scheduler builds from
-// the natural-key columns (recommendationID in internal/scheduler), not the
-// cache table's UUID primary key. Paging needs ID order to be a total order:
-// two rows sharing an ID would let the strict "after" comparison skip the
-// second one at a page boundary. A duplicate is therefore an error rather
-// than a silently short walk.
-//
-// Resuming after the last ID (rather than at an offset) means a row added or
-// removed between two requests does not shift later pages: the next page
-// always resumes after the last ID the client saw, even if that row has
-// since been deleted.
-func sortRecommendationsAfter(recs []config.RecommendationRecord, afterID string) (int, error) {
-	sort.Slice(recs, func(i, j int) bool { return recs[i].ID < recs[j].ID })
-	for i := 1; i < len(recs); i++ {
-		if recs[i].ID == recs[i-1].ID {
-			return 0, fmt.Errorf("duplicate recommendation ID %q: cannot page the recommendations list", recs[i].ID)
-		}
-	}
-	if afterID == "" {
-		return 0, nil
-	}
-	return sort.Search(len(recs), func(i int) bool { return recs[i].ID > afterID }), nil
-}
-
 // envelopeLen returns how many bytes v adds to the Lambda invoke response:
 // its JSON encoding, re-encoded as the content of the response's body
 // string (without the surrounding quotes).
@@ -194,42 +164,3 @@ func selectRecommendationsPage(sorted []config.RecommendationRecord, start, limi
 	}
 	return n, nil
 }
-
-// buildRecommendationsPageResponse builds the list response for either mode.
-// Unpaginated requests get exactly the legacy buildRecommendationsResponse
-// output; paginated requests get one page of rows plus next_cursor, with the
-// summary and regions still computed over the full filtered set.
-func buildRecommendationsPageResponse(recs []config.RecommendationRecord, page recommendationsPage) (*RecommendationsResponse, error) {
-	if !page.enabled {
-		return buildRecommendationsResponse(recs), nil
-	}
-	return buildBudgetedPage(recs, page, recommendationsPageByteBudget)
-}
-
-// buildBudgetedPage is the paginated branch of
-// buildRecommendationsPageResponse with an explicit byte budget.
-func buildBudgetedPage(recs []config.RecommendationRecord, page recommendationsPage, budget int) (*RecommendationsResponse, error) {
-	start, err := sortRecommendationsAfter(recs, page.afterID)
-	if err != nil {
-		return nil, err
-	}
-	resp := summarizeRecommendations(recs, recs[start:start])
-	// Overhead: everything except the rows and the cursor value, measured
-	// with an empty next_cursor key present.
-	resp.NextCursor = " "
-	overhead, err := envelopeLen(resp)
-	if err != nil {
-		return nil, fmt.Errorf("failed to size recommendations page: %w", err)
-	}
-	n, err := selectRecommendationsPage(recs, start, page.limit, overhead, budget)
-	if err != nil {
-		return nil, err
-	}
-	end := start + n
-	resp.Recommendations = recs[start:end]
-	resp.NextCursor = ""
-	if end < len(recs) {
-		resp.NextCursor = encodeRecommendationsCursor(recs[end-1].ID)
-	}
-	return resp, nil
-}

internal/api/handler_recommendations_page_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -43,10 +44,26 @@ func pageTestRecs(n int) []config.RecommendationRecord {
 func pageTestHandler(t *testing.T, recs []config.RecommendationRecord) *Handler {
 	t.Helper()
 	ms := new(MockScheduler)
+	// Unpaginated requests read the full list; paginated ones scan it in
+	// ID order (issue #981).
 	ms.On("ListRecommendations", mock.Anything, mock.Anything).Return(recs, nil)
+	ms.On("ScanRecommendations", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(recs, nil)
 	return &Handler{scheduler: ms, apiKey: "test-key"}
 }
 
+// buildBudgetedPage pages recs the way a paginated request does: the rows
+// reach the collector in ID order, as the store scan delivers them, and the
+// page is cut with the given byte budget.
+func buildBudgetedPage(recs []config.RecommendationRecord, page recommendationsPage, budget int) (*RecommendationsResponse, error) {
+	sorted := append([]config.RecommendationRecord(nil), recs...)
+	sort.SliceStable(sorted, func(i, j int) bool { return sorted[i].ID < sorted[j].ID })
+	collector := newRecommendationsPageCollector(page)
+	if err := collector.add(sorted); err != nil {
+		return nil, err
+	}
+	return collector.response(budget)
+}
+
 func pageTestReq() *events.LambdaFunctionURLRequest {
 	return &events.LambdaFunctionURLRequest{Headers: map[string]string{"x-api-key": "test-key"}}
 }