fix(dashboard): apply provider filter to savings-history chart (closes #498)

frontend/src/__tests__/dashboard.test.ts

@@ -840,6 +840,13 @@ describe('Dashboard Module', () => {
       b30.textContent = '30d';
       document.body.replaceChildren(canvas, empty, b90, b30);
       (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });
+      // Reset the topbar filter mocks to the documented default (no filter)
+      // so each test starts isolated. clearAllMocks() resets call history but
+      // not mockReturnValue implementations, so a provider/account value set by
+      // an earlier test would otherwise leak in (now that the chart reads the
+      // provider chip, issue #498).
+      (state.getCurrentProvider as jest.Mock).mockReturnValue('');
+      (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
     });
 
     test('uses daily interval for the default 90-day range', async () => {
@@ -1135,10 +1142,10 @@ describe('Dashboard Module', () => {
       expect(empty?.textContent).toContain('No purchase history yet');
     });
 
-    test('empty-state does NOT mention provider even when a provider filter is active (#764)', async () => {
-      // The analytics endpoint ignores the provider param until #764 lands.
-      // Showing "No savings history for aws." would imply the query was
-      // scoped to that provider, which is false. Generic copy is used instead.
+    test('empty-state names the provider when a provider filter is active (issue #498, QA 2.3)', async () => {
+      // The analytics endpoint now applies the provider param as a WHERE
+      // filter, so the query IS scoped to the selected provider — naming it
+      // in the empty-state is accurate. Mirrors the account-filter empty-state.
       (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
       (state.getCurrentProvider as jest.Mock).mockReturnValue('aws');
       (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });
@@ -1147,9 +1154,35 @@ describe('Dashboard Module', () => {
 
       const empty = document.getElementById('savings-trend-empty');
       expect(empty?.classList.contains('hidden')).toBe(false);
-      // Provider name must not appear in the message.
-      expect(empty?.textContent).not.toContain('aws');
-      expect(empty?.textContent).toContain('No purchase history yet');
+      // Provider name (uppercased by buildFilterDesc) appears in the message.
+      expect(empty?.textContent).toContain('AWS');
+      expect(empty?.textContent).toContain('No savings history for');
+    });
+
+    test('loadSavingsTrendChart forwards the provider chip to the analytics API (issue #498, QA 2.3)', async () => {
+      // Regression: previously the chart dropped the provider param, so the
+      // savings-history series showed the same all-provider data regardless of
+      // the selected provider. The fix forwards provider to scope the query.
+      (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
+      (state.getCurrentProvider as jest.Mock).mockReturnValue('aws');
+      (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });
+
+      await loadSavingsTrendChart();
+
+      expect(api.getSavingsAnalytics).toHaveBeenCalledWith(
+        expect.objectContaining({ provider: 'aws' })
+      );
+    });
+
+    test('loadSavingsTrendChart omits provider when no provider filter is active', async () => {
+      (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]);
+      (state.getCurrentProvider as jest.Mock).mockReturnValue('');
+      (api.getSavingsAnalytics as jest.Mock).mockResolvedValue({ data_points: [] });
+
+      await loadSavingsTrendChart();
+
+      const call = (api.getSavingsAnalytics as jest.Mock).mock.calls[0]?.[0];
+      expect(call).not.toHaveProperty('provider');
     });
   });
 

frontend/src/dashboard.ts

@@ -1033,20 +1033,23 @@ export async function loadSavingsTrendChart(): Promise<void> {
   const interval: 'hourly' | 'daily' | 'weekly' = intervalDays <= 7 ? 'hourly' : intervalDays <= 90 ? 'daily' : 'weekly';
 
   try {
-    // Always forward account_ids to the chart so its data scope matches the
-    // KPI tiles above it. The backend /history/analytics handler accepts a
-    // single `account_id`; api.getSavingsAnalytics also sets the singular
-    // param for single-account requests (see api/history.ts).
-    // provider is not forwarded: the analytics backend does not yet support
-    // provider-scoped queries (handler_analytics.go has no provider param).
+    // Always forward account_ids AND provider to the chart so its data scope
+    // matches the KPI tiles above it. The backend /history/analytics handler
+    // accepts a single `account_id`; api.getSavingsAnalytics also sets the
+    // singular param for single-account requests (see api/history.ts). The
+    // backend now applies the provider chip as a provider WHERE filter on the
+    // savings-history query, so forward it to scope the chart (issue #498,
+    // QA 2.3).
     const accountIDs = state.getCurrentAccountIDs();
+    const provider = state.getCurrentProvider();
     const data = await api.getSavingsAnalytics({
       // For 'all': send the epoch sentinel so the backend returns unbounded
       // history. Omitting start would cause parseDateRange to default to
       // (end - 7d), silently clipping the chart (see handler_analytics.go).
       start: isAllRange ? epochStart : new Date(windowStartMs).toISOString(),
       end: now.toISOString(),
       interval,
+      ...(provider ? { provider } : {}),
       ...(accountIDs.length > 0 ? { account_ids: accountIDs } : {}),
     });
 
@@ -1087,18 +1090,13 @@ export async function loadSavingsTrendChart(): Promise<void> {
     if (points.length === 0) {
       canvas.classList.add('hidden');
       if (empty) {
-        if (accountIDs.length > 0) {
-          // Account IDs are forwarded to the backend (see call above), so
-          // mentioning them in the empty-state is accurate.
-          empty.textContent = `No savings history for ${accountIDs.join(', ')}.`;
-        } else {
-          // Provider is intentionally NOT mentioned here: the analytics
-          // endpoint does not accept a provider param yet (tracked in #764),
-          // so the query always returns all-provider data regardless of the
-          // topbar provider filter. Claiming provider scope would be
-          // misleading — drop it until #764 lands.
-          empty.textContent = 'No purchase history yet.';
-        }
+        // Both provider and account are forwarded to the backend (see call
+        // above), so naming the active filter in the empty-state is accurate
+        // (issue #498). buildFilterDesc returns '' when no filter is active.
+        const filterDesc = buildFilterDesc(provider, accountIDs);
+        empty.textContent = filterDesc
+          ? `No savings history for ${filterDesc}.`
+          : 'No purchase history yet.';
         empty.classList.remove('hidden');
       }
       if (savingsTrendChart) { savingsTrendChart.destroy(); savingsTrendChart = null; }

frontend/src/modules/savings-history.ts

@@ -88,8 +88,8 @@ export async function loadSavingsHistory(): Promise<void> {
     // is single-select, and the backend's /history/analytics takes a single
     // account_id (see handler_analytics.go), so we forward the only selected
     // ID, mirroring dashboard.ts loadSavingsTrendChart. The provider chip is
-    // forwarded too; the backend honours it once #502 lands (until then it is
-    // a harmless no-op param and account_ids does the filtering).
+    // forwarded too and the backend now applies it as a provider WHERE filter
+    // on the savings-history query (issue #498, QA 2.3).
     const currentProvider = state.getCurrentProvider();
     const currentAccountIDs = state.getCurrentAccountIDs();
 

internal/api/analytics_postgres.go

@@ -143,11 +143,15 @@ func sortedProviderKeys(m map[string][]string) []string {
 // QueryHistory aggregates purchase_history rows bucketed by interval.
 // Empty accountUUIDs AND accountExternalIDsByProvider means "all accounts
 // accessible to the caller"; scoping is enforced upstream in the handler.
-// Returns data points in ascending order and a summary covering the full window.
+// A non-empty provider ("aws"/"azure"/"gcp") restricts to rows of that
+// provider; "" means all providers (the global-filter "all" sentinel is
+// normalised to "" in the handler). Returns data points in ascending order
+// and a summary covering the full window.
 func (c *PostgresAnalyticsClient) QueryHistory(
 	ctx context.Context,
 	accountUUIDs []string,
 	accountExternalIDsByProvider map[string][]string,
+	provider string,
 	start, end time.Time,
 	interval string,
 ) ([]HistoryDataPoint, *HistorySummary, error) {
@@ -159,11 +163,17 @@ func (c *PostgresAnalyticsClient) QueryHistory(
 	// Single query grouped by (bucket, service, provider) so we can assemble
 	// both the top-line bucket totals and the by_service / by_provider
 	// breakdowns without a second trip to the DB. The account predicate is the
-	// shared dual-column clause (see accountFilterClause).
+	// shared dual-column clause (see accountFilterClause); the optional provider
+	// predicate mirrors QueryByService (parameter-bound, "" = no filter).
 	//
 	// #nosec G201 — `unit` is allowlisted by intervalToTruncUnit above and the
-	// account clause is parameter-bound (no user input interpolated).
+	// account / provider clauses are parameter-bound (no user input interpolated).
 	accountClause, args := accountFilterClause(accountUUIDs, accountExternalIDsByProvider, []any{start, end})
+	providerClause := ""
+	if provider != "" {
+		args = append(args, provider)
+		providerClause = fmt.Sprintf(" AND provider = $%d", len(args))
+	}
 	query := fmt.Sprintf(`
 		SELECT date_trunc('%s', timestamp) AS bucket,
 		       service,
@@ -174,10 +184,10 @@ func (c *PostgresAnalyticsClient) QueryHistory(
 		FROM purchase_history
 		WHERE timestamp >= $1
 		  AND timestamp <= $2
-		  AND %s
+		  AND %s%s
 		GROUP BY bucket, service, provider
 		ORDER BY bucket ASC
-	`, unit, accountClause)
+	`, unit, accountClause, providerClause)
 
 	rows, err := c.db.Query(ctx, query, args...)
 	if err != nil {

internal/api/analytics_postgres_test.go

@@ -72,7 +72,7 @@ func TestQueryHistory_Success(t *testing.T) {
 
 	start := time.Date(2026, 4, 20, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 4, 24, 0, 0, 0, 0, time.UTC)
-	points, summary, err := client.QueryHistory(ctx, nil, map[string][]string{"": {"acct-1"}}, start, end, "daily")
+	points, summary, err := client.QueryHistory(ctx, nil, map[string][]string{"": {"acct-1"}}, "", start, end, "daily")
 	require.NoError(t, err)
 	require.Len(t, points, 2)
 
@@ -103,7 +103,7 @@ func TestQueryHistory_Success(t *testing.T) {
 
 func TestQueryHistory_BadInterval(t *testing.T) {
 	client, _ := newMockAnalyticsClient(t)
-	_, _, err := client.QueryHistory(context.Background(), nil, nil, time.Now(), time.Now(), "yearly")
+	_, _, err := client.QueryHistory(context.Background(), nil, nil, "", time.Now(), time.Now(), "yearly")
 	assert.Error(t, err)
 }
 
@@ -113,7 +113,7 @@ func TestQueryHistory_QueryError(t *testing.T) {
 	mock.ExpectQuery(`SELECT date_trunc`).
 		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
 		WillReturnError(errors.New("db down"))
-	_, _, err := client.QueryHistory(context.Background(), nil, nil, time.Now(), time.Now(), "daily")
+	_, _, err := client.QueryHistory(context.Background(), nil, nil, "", time.Now(), time.Now(), "daily")
 	assert.Error(t, err)
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
@@ -191,7 +191,7 @@ func TestQueryHistory_DualColumnFilter(t *testing.T) {
 
 	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
-	points, summary, err := client.QueryHistory(ctx, []string{uuid}, map[string][]string{"aws": {external}}, start, end, "daily")
+	points, summary, err := client.QueryHistory(ctx, []string{uuid}, map[string][]string{"aws": {external}}, "", start, end, "daily")
 	require.NoError(t, err)
 	require.Len(t, points, 1)
 	assert.InDelta(t, 50.0, points[0].TotalSavings, 1e-9)
@@ -244,7 +244,64 @@ func TestQueryHistory_CrossProviderScoped(t *testing.T) {
 
 	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
-	_, _, err := client.QueryHistory(ctx, nil, map[string][]string{"aws": {"123"}, "azure": {"123"}}, start, end, "daily")
+	_, _, err := client.QueryHistory(ctx, nil, map[string][]string{"aws": {"123"}, "azure": {"123"}}, "", start, end, "daily")
+	require.NoError(t, err)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestQueryHistory_ProviderFilter is the regression for the Savings History
+// chart provider-filter bug (issue #498, QA 2.3): selecting a provider in the
+// global filter must scope the savings-over-time series to that provider. The
+// handler forwards the chip value, so QueryHistory must append
+// `AND provider = $N` bound to the provider so Azure/GCP rows are excluded.
+// Pre-fix QueryHistory ignored the provider entirely and this test fails
+// because no provider bind is emitted.
+func TestQueryHistory_ProviderFilter(t *testing.T) {
+	client, mock := newMockAnalyticsClient(t)
+	ctx := context.Background()
+
+	bucket := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)
+	// Only the aws row comes back from the DB; the provider predicate is what
+	// keeps azure/gcp rows out, so the mock returns just the aws bucket.
+	rows := mock.NewRows([]string{"bucket", "service", "provider", "savings", "upfront", "purchases"}).
+		AddRow(bucket, "ec2", "aws", 10.0, 5.0, 1)
+
+	// No account filter (all-accessible), so the account clause degrades to TRUE
+	// and start/end are $1/$2; the provider bind is therefore $3.
+	mock.ExpectQuery(`(?s)SELECT date_trunc\('day', timestamp\).*AND TRUE AND provider = \$3`).
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), "aws").
+		WillReturnRows(rows)
+
+	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
+	points, _, err := client.QueryHistory(ctx, nil, nil, "aws", start, end, "daily")
+	require.NoError(t, err)
+	require.Len(t, points, 1)
+	assert.InDelta(t, 10.0, points[0].TotalSavings, 1e-9)
+	assert.NoError(t, mock.ExpectationsWereMet())
+}
+
+// TestQueryHistory_ProviderFilterWithAccount verifies the provider bind is
+// positioned after the dual-column account binds (provider becomes $6 when a
+// UUID + provider-grouped external id precede it), so account + provider
+// filtering compose correctly (issue #498).
+func TestQueryHistory_ProviderFilterWithAccount(t *testing.T) {
+	client, mock := newMockAnalyticsClient(t)
+	ctx := context.Background()
+	uuid := "aabbccdd-1234-5678-abcd-aabbccddee00"
+	external := "123456789012"
+
+	bucket := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)
+	rows := mock.NewRows([]string{"bucket", "service", "provider", "savings", "upfront", "purchases"}).
+		AddRow(bucket, "ec2", "aws", 50.0, 20.0, 1)
+
+	mock.ExpectQuery(`(?s)SELECT date_trunc\('day', timestamp\).*AND \(cloud_account_id = ANY\(\$3\) OR \(provider = \$4 AND account_id = ANY\(\$5\)\)\) AND provider = \$6`).
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), []string{uuid}, "aws", []string{external}, "aws").
+		WillReturnRows(rows)
+
+	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
+	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
+	_, _, err := client.QueryHistory(ctx, []string{uuid}, map[string][]string{"aws": {external}}, "aws", start, end, "daily")
 	require.NoError(t, err)
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
@@ -267,7 +324,7 @@ func TestQueryHistory_ExternalIDFilter(t *testing.T) {
 
 	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
-	_, _, err := client.QueryHistory(ctx, nil, map[string][]string{"": {"123456789012"}}, start, end, "daily")
+	_, _, err := client.QueryHistory(ctx, nil, map[string][]string{"": {"123456789012"}}, "", start, end, "daily")
 	require.NoError(t, err)
 	assert.NoError(t, mock.ExpectationsWereMet())
 }

internal/api/handler_analytics.go

@@ -134,6 +134,18 @@ func (h *Handler) getHistoryAnalytics(ctx context.Context, req *events.LambdaFun
 		interval = "hourly"
 	}
 
+	// Normalise + validate the global-filter provider chip, mirroring
+	// parseHistoryFilters: "all" (and "") mean no provider filter, anything
+	// else must be a known provider. Without this the chart ignored the
+	// provider chip entirely (issue #498, QA 2.3).
+	provider := params["provider"]
+	if provider == "all" {
+		provider = "" // explicit "no filter" sentinel
+	}
+	if err := validateProvider(provider); err != nil {
+		return nil, err
+	}
+
 	// For scoped users we require account_id and validate it's in their scope.
 	// We don't (yet) support analytics across a subset — the underlying
 	// aggregate takes a single account_id. An unrestricted/admin session
@@ -154,7 +166,7 @@ func (h *Handler) getHistoryAnalytics(ctx context.Context, req *events.LambdaFun
 	accountUUIDs, accountExternalIDsByProvider := h.resolveSingleAccountFilterIDs(ctx, accountID)
 
 	// Aggregate history from the analytics client (Postgres-backed).
-	dataPoints, summary, err := h.analyticsClient.QueryHistory(ctx, accountUUIDs, accountExternalIDsByProvider, start, end, interval)
+	dataPoints, summary, err := h.analyticsClient.QueryHistory(ctx, accountUUIDs, accountExternalIDsByProvider, provider, start, end, interval)
 	if err != nil {
 		return nil, fmt.Errorf("failed to query analytics: %w", err)
 	}