feat(recommendations): server-side pushdown for service/region/account filters (closes #162)

frontend/src/__tests__/recommendations.test.ts

@@ -157,10 +157,11 @@ describe('Recommendations Module', () => {
   });
 
   describe('loadRecommendations', () => {
-    test('fetches recommendations with provider/account_ids hints (Bundle B)', async () => {
-      // After Bundle B, only provider + account_ids are sent to the API as
-      // hints; service/region/numeric filters are pure client-side via
-      // applyColumnFilters. The legacy DOM filter inputs are gone.
+    afterEach(() => (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({}));
+
+    test('fetches with provider/account_ids when no column filter is set', async () => {
+      // No column filters active: service and region are absent from the
+      // API call (undefined); provider + account_ids carry the only hints.
       (api.getRecommendations as jest.Mock).mockResolvedValue({
         summary: {},
         recommendations: [],
@@ -172,7 +173,63 @@ describe('Recommendations Module', () => {
       expect(api.getRecommendations).toHaveBeenCalledWith({
         provider: 'all',
         account_ids: undefined,
+        service: undefined,
+        region: undefined,
+      });
+    });
+
+    test('pushes single-value service filter to API params (issue #162)', async () => {
+      // A single-value service column filter must be forwarded to the API
+      // so the backend applies it in SQL, saving bandwidth on large tenants.
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({
+        service: { kind: 'set', values: ['ec2'] },
+      });
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: {},
+        recommendations: [],
+        regions: [],
+      });
+
+      await loadRecommendations();
+
+      expect(api.getRecommendations).toHaveBeenCalledWith(
+        expect.objectContaining({ service: 'ec2' }),
+      );
+    });
+
+    test('pushes single-value region filter to API params (issue #162)', async () => {
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({
+        region: { kind: 'set', values: ['us-east-1'] },
+      });
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: {},
+        recommendations: [],
+        regions: [],
       });
+
+      await loadRecommendations();
+
+      expect(api.getRecommendations).toHaveBeenCalledWith(
+        expect.objectContaining({ region: 'us-east-1' }),
+      );
+    });
+
+    test('does not push multi-value service filter to API (stays client-side)', async () => {
+      // When the user has selected multiple services the server only supports
+      // equality (single-value); fall back to client-side filtering.
+      (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({
+        service: { kind: 'set', values: ['ec2', 'rds'] },
+      });
+      (api.getRecommendations as jest.Mock).mockResolvedValue({
+        summary: {},
+        recommendations: [],
+        regions: [],
+      });
+
+      await loadRecommendations();
+
+      const call = (api.getRecommendations as jest.Mock).mock.calls[0][0];
+      expect(call.service).toBeUndefined();
     });
 
     test('renders recommendations summary', async () => {
@@ -4971,11 +5028,16 @@ describe('Monthly Cost + Effective % column rendering', () => {
     });
     (state.getRecommendations as jest.Mock).mockReturnValue([nullRec]);
     // Apply a filter that would match $0 if NaN were treated as 0.
-    // Use mockReturnValueOnce so the override doesn't leak into subsequent tests.
-    (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValueOnce({
+    // loadRecommendations now calls getRecommendationsColumnFilters once for
+    // pushdown extraction before the render path calls it again (issue #162),
+    // so we need mockReturnValue rather than mockReturnValueOnce. The mock is
+    // explicitly reset to {} afterward so subsequent tests are unaffected
+    // (jest.clearAllMocks does not reset mockReturnValue).
+    (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({
       on_demand_monthly: { kind: 'expr', expr: '0' },
     });
     await loadRecommendations();
+    (state.getRecommendationsColumnFilters as jest.Mock).mockReturnValue({});
 
     // Null-monthly_cost row should be filtered out; tbody should have no visible recommendation rows.
     const rows = document.querySelectorAll('tbody tr.recommendation-row');

frontend/src/recommendations.ts

@@ -195,8 +195,10 @@ export function resetCachedGlobalConfig(): void {
 // populateRecommendationsAccountFilter / populateRegionFilter / the legacy
 // service-filter helpers were removed in Bundle B — those DOM elements are
 // gone from index.html. Provider / Account values now drive an API re-fetch
-// via the column-header popovers (see openColumnPopover wiring); Service /
-// Region filtering is purely client-side via applyColumnFilters.
+// via the column-header popovers (see openColumnPopover wiring). Service and
+// Region single-value categorical commits also trigger a re-fetch (issue
+// #162) so the backend WHERE clause prunes rows before they hit the wire;
+// multi-value and numeric column filters remain purely client-side.
 
 /**
  * Get the recommendations currently loaded in the purchase modal,
@@ -488,13 +490,31 @@ export async function loadRecommendations(): Promise<void> {
   }
 
   try {
-    // Provider + account_ids are still sent to the API as hints so the
-    // backend stays bounded for big multi-cloud tenants. Service / Region /
-    // numeric filters are pure client-side via applyColumnFilters.
+    // Provider + account_ids are sent to the API as push-down hints so the
+    // backend stays bounded for big multi-cloud tenants. Service and Region
+    // single-value categorical filters are also pushed down (issue #162):
+    // when the user has selected exactly one value in the service or region
+    // column popover the API receives that value and the backend applies the
+    // WHERE clause before returning rows, saving bandwidth on large tenants.
+    // Multi-value or absent service/region filters fall through unchanged and
+    // are applied client-side by applyColumnFilters below.
     const accountIDs = state.getCurrentAccountIDs();
+    const columnFilters = state.getRecommendationsColumnFilters();
+    const serviceFilter = columnFilters['service'];
+    const regionFilter = columnFilters['region'];
+    const pushedService =
+      serviceFilter?.kind === 'set' && serviceFilter.values.length === 1
+        ? serviceFilter.values[0]
+        : undefined;
+    const pushedRegion =
+      regionFilter?.kind === 'set' && regionFilter.values.length === 1
+        ? regionFilter.values[0]
+        : undefined;
     const filters: api.RecommendationFilters = {
       provider: state.getCurrentProvider(),
       account_ids: accountIDs.length > 0 ? accountIDs : undefined,
+      service: pushedService,
+      region: pushedRegion,
     };
 
     const [data, accounts, cfgResponse] = await Promise.all([
@@ -1967,6 +1987,34 @@ function categoricalDisplayLabel(
   return value;
 }
 
+// Columns whose single-value categorical filter is pushed down to the server
+// (issue #162). When a commit on one of these columns resolves to exactly one
+// selected value, loadRecommendations() re-fetches from the API with that
+// value as a query param; the backend applies it in the WHERE clause before
+// returning rows. For multi-value or cleared filters on these columns the
+// filter is still applied client-side so the UX stays consistent.
+// 'account' drives account_ids which is already a server-side hint; include
+// it here so clearing the account filter triggers a re-fetch like the others.
+const SERVER_PUSHDOWN_COLUMNS: ReadonlySet<state.RecommendationsColumnId> = new Set([
+  'service', 'region', 'account',
+]);
+
+// commitAndRefresh is called by every categorical popover commit. For
+// server-pushdown columns it fires loadRecommendations() (re-fetch) so the
+// backend can prune rows before they hit the wire; for all other columns it
+// calls rerenderRecommendations() (client-side filter, no extra API call).
+//
+// The re-fetch path is intentionally async/void: the popover commit returns
+// immediately and the table skeleton renders while the fetch is in flight,
+// matching the existing provider/account-change flow.
+function commitAndRefresh(column: state.RecommendationsColumnId): void {
+  if (SERVER_PUSHDOWN_COLUMNS.has(column)) {
+    void loadRecommendations();
+  } else {
+    rerenderRecommendations();
+  }
+}
+
 // Build the popover DOM for a given column. Categorical: checkbox list with
 // (All) tri-state + Clear footer. Numeric: free-text expression input with
 // inline error and Clear footer.
@@ -2131,6 +2179,9 @@ function buildPopoverContent(
     //     allow-list so unchecking the last value reaches the same
     //     zero-row state as the (All) checkbox and the Clear button,
     //     rather than snapping back to "all checked".
+    // For server-pushdown columns (service / region / account) a single-value
+    // selection triggers a re-fetch so the backend prunes rows before they
+    // hit the wire (issue #162). All other columns stay client-side only.
     const commit = (): void => {
       const selected: string[] = [];
       checkboxes.forEach((cb, value) => { if (cb.checked) selected.push(value); });
@@ -2142,7 +2193,7 @@ function buildPopoverContent(
       saveColumnFilters(state.getRecommendationsColumnFilters());
       updateAllTriState();
       updateSPTriState();
-      rerenderRecommendations();
+      commitAndRefresh(column);
     };
 
     // (All) and Clear use commitAll(target) directly:
@@ -2161,7 +2212,7 @@ function buildPopoverContent(
       saveColumnFilters(state.getRecommendationsColumnFilters());
       updateAllTriState();
       updateSPTriState();
-      rerenderRecommendations();
+      commitAndRefresh(column);
     };
     // Expose commitAll to the Clear button handler outside this else-branch.
     commitAllRef = commitAll;
@@ -2214,7 +2265,8 @@ function buildPopoverContent(
       // Issue #700: call commitAllRef(false) (the same as commitAll(false)
       // inside the categorical branch) so updateAllTriState() is invoked and
       // the (All) checkbox reflects the cleared state. commitAllRef also
-      // calls rerenderRecommendations() internally.
+      // calls commitAndRefresh(column) internally, which re-fetches for
+      // server-pushdown columns (issue #162) or re-renders for the others.
       commitAllRef?.(false);
     }
   });

internal/api/handler_recommendations_test.go

@@ -43,6 +43,82 @@ func TestHandler_getRecommendations(t *testing.T) {
 	assert.Equal(t, float64(0), result.Summary.TotalMonthlySavings)
 }
 
+// TestGetRecommendations_PushdownFilters verifies that service, region, and
+// account_id query params are translated into the RecommendationFilter that
+// reaches ListRecommendations (issue #162). The mock is registered with the
+// exact filter shape expected so testify's mock.AssertExpectations confirms
+// the backend does not silently drop any of the three values.
+func TestGetRecommendations_PushdownFilters(t *testing.T) {
+	ctx := context.Background()
+	validUUID := "12345678-1234-1234-1234-123456789abc"
+
+	t.Run("service and region params reach the filter", func(t *testing.T) {
+		mockScheduler := new(MockScheduler)
+		t.Cleanup(func() { mockScheduler.AssertExpectations(t) })
+		mockScheduler.On(
+			"ListRecommendations", ctx,
+			config.RecommendationFilter{Provider: "aws", Service: "ec2", Region: "us-east-1"},
+		).Return([]config.RecommendationRecord{}, nil)
+
+		handler := &Handler{scheduler: mockScheduler, apiKey: "test-key"}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "test-key"},
+		}
+		_, err := handler.getRecommendations(ctx, req, map[string]string{
+			"provider": "aws",
+			"service":  "ec2",
+			"region":   "us-east-1",
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("account_id param reaches the filter", func(t *testing.T) {
+		mockScheduler := new(MockScheduler)
+		t.Cleanup(func() { mockScheduler.AssertExpectations(t) })
+		mockScheduler.On(
+			"ListRecommendations", ctx,
+			config.RecommendationFilter{AccountIDs: []string{validUUID}},
+		).Return([]config.RecommendationRecord{}, nil)
+
+		handler := &Handler{scheduler: mockScheduler, apiKey: "test-key"}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "test-key"},
+		}
+		_, err := handler.getRecommendations(ctx, req, map[string]string{
+			"account_ids": validUUID,
+		})
+		require.NoError(t, err)
+	})
+
+	t.Run("invalid service slug returns 400", func(t *testing.T) {
+		handler := &Handler{apiKey: "test-key"}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "test-key"},
+		}
+		_, err := handler.getRecommendations(ctx, req, map[string]string{
+			"service": "EC2 Reserved!", // uppercase + space + bang are invalid
+		})
+		require.Error(t, err)
+		ce, ok := IsClientError(err)
+		require.True(t, ok, "expected ClientError, got %T", err)
+		assert.Equal(t, 400, ce.code)
+	})
+
+	t.Run("invalid region name returns 400", func(t *testing.T) {
+		handler := &Handler{apiKey: "test-key"}
+		req := &events.LambdaFunctionURLRequest{
+			Headers: map[string]string{"x-api-key": "test-key"},
+		}
+		_, err := handler.getRecommendations(ctx, req, map[string]string{
+			"region": "US EAST 1!", // spaces and bang are invalid
+		})
+		require.Error(t, err)
+		ce, ok := IsClientError(err)
+		require.True(t, ok, "expected ClientError, got %T", err)
+		assert.Equal(t, 400, ce.code)
+	})
+}
+
 func TestHandler_getRecommendationsFreshness(t *testing.T) {
 	ctx := context.Background()