chore(common): rename ServiceSavingsPlans -> ServiceSavingsPlansAll (closes #786)

cmd/helpers_test.go

@@ -308,7 +308,7 @@ func TestApplyCoverage(t *testing.T) {
 			name: "Savings Plans - reduces hourly commitment",
 			recs: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					Count:            1,
 					EstimatedSavings: 100,
 					Details: &common.SavingsPlanDetails{

cmd/main_test.go

@@ -241,12 +241,12 @@ func TestCreateServiceClient(t *testing.T) {
 			expectNil: false,
 		},
 		{
-			// Legacy umbrella slug is no longer dispatched to a client — the
+			// Umbrella sentinel is no longer dispatched to a client — the
 			// per-plan-type slugs above carry the actual work. Keep the case
 			// to lock the contract: createServiceClient returns nil for the
 			// umbrella so the caller knows to fan out.
-			name:      "Savings Plans umbrella service (legacy, returns nil)",
-			service:   common.ServiceSavingsPlans,
+			name:      "Savings Plans umbrella sentinel (returns nil)",
+			service:   common.ServiceSavingsPlansAll,
 			expectNil: true,
 		},
 		{

cmd/multi_service_helpers.go

@@ -58,7 +58,7 @@ func getServiceDisplayName(service common.ServiceType) string {
 // one-line change.
 func savingsPlanDisplayName(service common.ServiceType) (string, bool) {
 	labels := map[common.ServiceType]string{
-		common.ServiceSavingsPlans:            "Savings Plans",
+		common.ServiceSavingsPlansAll:         "Savings Plans",
 		common.ServiceSavingsPlansCompute:     "Compute Savings Plans",
 		common.ServiceSavingsPlansEC2Instance: "EC2 Instance Savings Plans",
 		common.ServiceSavingsPlansSageMaker:   "SageMaker Savings Plans",

cmd/multi_service_stats.go

@@ -112,7 +112,7 @@ func separateAndAggregateStats(serviceStats map[common.ServiceType]ServiceProces
 	for service, stats := range serviceStats {
 		if common.IsSavingsPlan(service) {
 			if spStats.Service == "" {
-				spStats.Service = common.ServiceSavingsPlans
+				spStats.Service = common.ServiceSavingsPlansAll
 			}
 			if stats.RegionsProcessed > spStats.RegionsProcessed {
 				spStats.RegionsProcessed = stats.RegionsProcessed

cmd/multi_service_stats_test.go

@@ -273,7 +273,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 			name: "Prints Compute Savings Plans",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 500.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "Compute",
@@ -282,7 +282,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 				},
 			},
 			stats: ServiceProcessingStats{
-				Service:                 common.ServiceSavingsPlans,
+				Service:                 common.ServiceSavingsPlansAll,
 				RecommendationsSelected: 1,
 				TotalEstimatedSavings:   500.0,
 			},
@@ -296,7 +296,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 			name: "Prints EC2 Instance Savings Plans",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 300.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "EC2Instance",
@@ -305,7 +305,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 				},
 			},
 			stats: ServiceProcessingStats{
-				Service:                 common.ServiceSavingsPlans,
+				Service:                 common.ServiceSavingsPlansAll,
 				RecommendationsSelected: 1,
 				TotalEstimatedSavings:   300.0,
 			},
@@ -318,7 +318,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 			name: "Prints Database Savings Plans",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 400.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "Database",
@@ -327,7 +327,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 				},
 			},
 			stats: ServiceProcessingStats{
-				Service:                 common.ServiceSavingsPlans,
+				Service:                 common.ServiceSavingsPlansAll,
 				RecommendationsSelected: 1,
 				TotalEstimatedSavings:   400.0,
 			},
@@ -340,7 +340,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 			name: "Prints SageMaker Savings Plans",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 250.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "SageMaker",
@@ -349,7 +349,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 				},
 			},
 			stats: ServiceProcessingStats{
-				Service:                 common.ServiceSavingsPlans,
+				Service:                 common.ServiceSavingsPlansAll,
 				RecommendationsSelected: 1,
 				TotalEstimatedSavings:   250.0,
 			},
@@ -362,15 +362,15 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 			name: "Prints multiple SP types with recommendations",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 500.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "Compute",
 						HourlyCommitment: 1.5,
 					},
 				},
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 600.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType:         "EC2Instance",
@@ -379,7 +379,7 @@ func TestPrintSavingsPlansSection(t *testing.T) {
 				},
 			},
 			stats: ServiceProcessingStats{
-				Service:                 common.ServiceSavingsPlans,
+				Service:                 common.ServiceSavingsPlansAll,
 				RecommendationsSelected: 2,
 				TotalEstimatedSavings:   1100.0,
 			},
@@ -416,7 +416,7 @@ func TestPrintComparisonSection(t *testing.T) {
 			name: "Comparison with EC2 RIs and EC2 Instance SP",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 600.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType: "EC2Instance",
@@ -440,7 +440,7 @@ func TestPrintComparisonSection(t *testing.T) {
 			name: "Comparison with Database RIs and Database SP",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 800.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType: "Database",
@@ -466,14 +466,14 @@ func TestPrintComparisonSection(t *testing.T) {
 			name: "Compute SP better than EC2 Instance SP",
 			recommendations: []common.Recommendation{
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 500.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType: "EC2Instance",
 					},
 				},
 				{
-					Service:          common.ServiceSavingsPlans,
+					Service:          common.ServiceSavingsPlansAll,
 					EstimatedSavings: 700.0,
 					Details: &common.SavingsPlanDetails{
 						PlanType: "Compute",

cmd/multi_service_test.go

@@ -820,8 +820,8 @@ func TestApplyFilters_RegionFiltering(t *testing.T) {
 		{
 			name: "Savings Plans bypass region filter",
 			recs: []common.Recommendation{
-				{Region: "us-east-1", ResourceType: "ComputeSP", Count: 1, Service: common.ServiceSavingsPlans},
-				{Region: "us-west-2", ResourceType: "ComputeSP", Count: 2, Service: common.ServiceSavingsPlans},
+				{Region: "us-east-1", ResourceType: "ComputeSP", Count: 1, Service: common.ServiceSavingsPlansAll},
+				{Region: "us-west-2", ResourceType: "ComputeSP", Count: 2, Service: common.ServiceSavingsPlansAll},
 			},
 			includeRegions: []string{},
 			excludeRegions: []string{},

internal/purchase/coverage_extra_test.go

@@ -50,14 +50,14 @@ func TestMapServiceType_AllBranches(t *testing.T) {
 		{"opensearch", common.ServiceOpenSearch},
 		{"redshift", common.ServiceRedshift},
 		{"memorydb", common.ServiceMemoryDB},
-		{"savingsplans", common.ServiceSavingsPlans},
+		{"savingsplans", common.ServiceSavingsPlansAll},
 		// Issue #85: the legacy hyphenated form is still accepted as a
 		// backwards-compat alias so Lambda-scheduled purchase executions
 		// persisted before the normalisation (rec.Service == "savings-plans"
 		// in purchase_executions.recommendations JSONB) still map correctly
 		// when re-executed. Once historical rows have aged out, the alias
 		// arm can be dropped and this case should flip to ServiceType("savings-plans").
-		{"savings-plans", common.ServiceSavingsPlans},
+		{"savings-plans", common.ServiceSavingsPlansAll},
 		{"unknown-service", common.ServiceType("unknown-service")},
 		{"", common.ServiceType("")},
 	}
@@ -1100,7 +1100,7 @@ func TestManager_ExecuteSinglePurchase_DetailsByService(t *testing.T) {
 			// details" on legacy rows, re-introducing #453 for SP umbrella.
 			name:        "sp_legacy_umbrella",
 			service:     "savings-plans",
-			serviceType: common.ServiceSavingsPlans,
+			serviceType: common.ServiceSavingsPlansAll,
 			region:      "us-east-1",
 			resource:    "LegacySP",
 			details:     &common.SavingsPlanDetails{PlanType: "Compute", HourlyCommitment: 0.75},

internal/purchase/execution.go

@@ -1016,8 +1016,8 @@ func mapServiceSlug(service string) (common.ServiceType, bool) {
 // frontend-canonical normalisation.
 func mapSavingsPlansSlug(service string) (common.ServiceType, bool) {
 	slugs := map[string]common.ServiceType{
-		"savings-plans":             common.ServiceSavingsPlans,
-		"savingsplans":              common.ServiceSavingsPlans,
+		"savings-plans":             common.ServiceSavingsPlansAll,
+		"savingsplans":              common.ServiceSavingsPlansAll,
 		"savings-plans-compute":     common.ServiceSavingsPlansCompute,
 		"savingsplans-compute":      common.ServiceSavingsPlansCompute,
 		"savings-plans-ec2instance": common.ServiceSavingsPlansEC2Instance,

pkg/common/service_details_codec.go

@@ -121,14 +121,14 @@ func newDetailsForService(service string) (ServiceDetails, bool) {
 
 	// AWS Savings Plans — all umbrella + per-plan-type slugs use
 	// SavingsPlanDetails. The dash-free spellings are the canonical
-	// values of ServiceSavingsPlans / ServiceSavingsPlansCompute etc.;
+	// values of ServiceSavingsPlansAll / ServiceSavingsPlansCompute etc.;
 	// "savings-plans" (with a dash) is the legacy umbrella alias that
 	// internal/purchase/execution.go: mapSavingsPlansSlug still
 	// recognises so purchase_executions JSONB rows persisted before
 	// the rename in PR #94 still resolve. Recognising it here means a
 	// legacy direct-execute approval still decodes against the right
 	// type.
-	case string(ServiceSavingsPlans),
+	case string(ServiceSavingsPlansAll),
 		string(ServiceSavingsPlansCompute),
 		string(ServiceSavingsPlansEC2Instance),
 		string(ServiceSavingsPlansSageMaker),

pkg/common/service_details_codec_test.go

@@ -143,7 +143,7 @@ func TestDecodeServiceDetailsFor_LegacyEmpty(t *testing.T) {
 		string(ServiceEC2),
 		string(ServiceRDS),
 		string(ServiceElastiCache),
-		string(ServiceSavingsPlans),
+		string(ServiceSavingsPlansAll),
 		string(ServiceSavingsPlansCompute),
 		string(ServiceOpenSearch),
 		string(ServiceRedshift),

pkg/common/types.go

@@ -56,16 +56,21 @@ const (
 
 	// Savings/Commitments
 	//
-	// ServiceSavingsPlans is the canonical umbrella identifier for AWS Savings
-	// Plans. The string value "savingsplans" (no hyphen) matches the frontend's
-	// identifier and the value persisted in service_configs.service /
-	// purchase_history.service so that direct comparisons
-	// (rec.Service == ServiceSavingsPlans) work without a normaliser. See
-	// issue #85 for the rationale (frontend chosen as canonical to avoid a
-	// SQL data migration). Code that needs to recognise pre-#85 persisted
-	// "savings-plans" rows (e.g. purchase_executions JSONB blobs) goes
-	// through the mapper in internal/purchase/execution.go.
-	ServiceSavingsPlans ServiceType = "savingsplans" // AWS Savings Plans (umbrella)
+	// ServiceSavingsPlansAll is the umbrella sentinel for all AWS Savings Plans.
+	// Its string value "savingsplans" (no hyphen) matches the frontend identifier
+	// and the value persisted in service_configs.service / purchase_history.service
+	// so that direct comparisons work without a normaliser. See issue #85 for the
+	// rationale (frontend chosen as canonical to avoid a SQL data migration).
+	//
+	// This is NOT a per-plan-type slug. The four granular forms are:
+	//   ServiceSavingsPlansCompute, ServiceSavingsPlansEC2Instance,
+	//   ServiceSavingsPlansSageMaker, ServiceSavingsPlansDatabase.
+	// Use SavingsPlansPlanTypes() to iterate over them in canonical order.
+	//
+	// Code that needs to recognise pre-#85 persisted "savings-plans" rows
+	// (e.g. purchase_executions JSONB blobs) goes through the mapper in
+	// internal/purchase/execution.go.
+	ServiceSavingsPlansAll ServiceType = "savingsplans" // umbrella sentinel for "any SP"
 
 	// Per-plan-type Savings Plans slugs. Each maps 1:1 to an AWS
 	// types.SupportedSavingsPlansType so users can configure term/payment
@@ -100,15 +105,15 @@ func (s ServiceType) String() string {
 	return string(s)
 }
 
-// IsSavingsPlan reports whether s is any Savings Plans service slug —
-// the legacy umbrella (ServiceSavingsPlans), any of the four per-plan-type
+// IsSavingsPlan reports whether s is any Savings Plans service slug --
+// the umbrella sentinel (ServiceSavingsPlansAll), any of the four per-plan-type
 // constants, or the dash-free frontend spelling "savingsplans" that the API
 // handler stores verbatim without normalisation. Use it when code needs to
 // recognise the Savings Plans family irrespective of plan type (e.g., stats
 // aggregation, region-ignoring filters, display-name branching).
 func IsSavingsPlan(s ServiceType) bool {
 	switch s {
-	case ServiceSavingsPlans,
+	case ServiceSavingsPlansAll,
 		ServiceSavingsPlansCompute,
 		ServiceSavingsPlansEC2Instance,
 		ServiceSavingsPlansSageMaker,
@@ -118,6 +123,20 @@ func IsSavingsPlan(s ServiceType) bool {
 	return string(s) == "savingsplans"
 }
 
+// SavingsPlansPlanTypes returns the four per-plan-type SP slugs in canonical
+// iteration order (Compute -> EC2Instance -> SageMaker -> Database). Use this
+// for any fan-out over "all SP plan types"; it makes the 4-way expansion
+// visible at the call site instead of relying on the umbrella sentinel
+// (ServiceSavingsPlansAll) to expand implicitly inside the rec collector.
+func SavingsPlansPlanTypes() []ServiceType {
+	return []ServiceType{
+		ServiceSavingsPlansCompute,
+		ServiceSavingsPlansEC2Instance,
+		ServiceSavingsPlansSageMaker,
+		ServiceSavingsPlansDatabase,
+	}
+}
+
 // CommitmentType represents different commitment types across clouds
 type CommitmentType string
 
@@ -521,7 +540,7 @@ type SavingsPlanDetails struct {
 }
 
 func (d SavingsPlanDetails) GetServiceType() ServiceType {
-	return ServiceSavingsPlans
+	return ServiceSavingsPlansAll
 }
 
 func (d SavingsPlanDetails) GetDetailDescription() string {

pkg/common/types_test.go

@@ -41,9 +41,9 @@ func TestServiceType_String(t *testing.T) {
 		{ServiceStorage, "storage"},
 		// Regression guard for issue #85: the frontend persists "savingsplans"
 		// (no hyphen) and the Go constant must match so direct comparisons of
-		// rec.Service == ServiceSavingsPlans don't silently miss rows. If you
+		// rec.Service == ServiceSavingsPlansAll don't silently miss rows. If you
 		// flip this back to "savings-plans" you also need a SQL migration.
-		{ServiceSavingsPlans, "savingsplans"},
+		{ServiceSavingsPlansAll, "savingsplans"},
 		{ServiceSavingsPlansCompute, "savings-plans-compute"},
 		{ServiceSavingsPlansEC2Instance, "savings-plans-ec2instance"},
 		{ServiceSavingsPlansSageMaker, "savings-plans-sagemaker"},
@@ -73,7 +73,7 @@ func TestIsSavingsPlan(t *testing.T) {
 		service ServiceType
 		want    bool
 	}{
-		{"legacy umbrella", ServiceSavingsPlans, true},
+		{"umbrella sentinel", ServiceSavingsPlansAll, true},
 		{"compute", ServiceSavingsPlansCompute, true},
 		{"ec2 instance", ServiceSavingsPlansEC2Instance, true},
 		{"sagemaker", ServiceSavingsPlansSageMaker, true},
@@ -90,6 +90,27 @@ func TestIsSavingsPlan(t *testing.T) {
 	}
 }
 
+func TestSavingsPlansPlanTypes(t *testing.T) {
+	t.Parallel()
+	got := SavingsPlansPlanTypes()
+
+	// Must return exactly 4 plan types.
+	assert.Len(t, got, 4)
+
+	// Canonical order: Compute, EC2Instance, SageMaker, Database.
+	assert.Equal(t, []ServiceType{
+		ServiceSavingsPlansCompute,
+		ServiceSavingsPlansEC2Instance,
+		ServiceSavingsPlansSageMaker,
+		ServiceSavingsPlansDatabase,
+	}, got)
+
+	// Every returned slug must be recognised as a Savings Plan.
+	for _, st := range got {
+		assert.Truef(t, IsSavingsPlan(st), "IsSavingsPlan(%q) should be true", st)
+	}
+}
+
 func TestCommitmentType_String(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
@@ -311,7 +332,7 @@ func TestSavingsPlanDetails_GetServiceType(t *testing.T) {
 		HourlyCommitment: 10.50,
 	}
 
-	assert.Equal(t, ServiceSavingsPlans, details.GetServiceType())
+	assert.Equal(t, ServiceSavingsPlansAll, details.GetServiceType())
 }
 
 func TestSavingsPlanDetails_GetDetailDescription(t *testing.T) {