From 3c5fce39f2c9fa09d54a015678304c89c118cb4d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Thu, 28 May 2026 16:20:15 +0200 Subject: [PATCH 1/3] chore(common): rename ServiceSavingsPlans -> ServiceSavingsPlansAll (closes #786) Add SavingsPlansPlanTypes() helper returning the 4 per-plan-type slugs in canonical order. Update IsSavingsPlan switch to use the new name. String value "savingsplans" unchanged (DB column locked by issue #85). --- cmd/helpers_test.go | 2 +- cmd/main_test.go | 6 +-- cmd/multi_service_helpers.go | 2 +- cmd/multi_service_stats.go | 2 +- cmd/multi_service_stats_test.go | 30 ++++++------ cmd/multi_service_test.go | 4 +- internal/purchase/coverage_extra_test.go | 6 +-- internal/purchase/execution.go | 4 +- pkg/common/service_details_codec.go | 4 +- pkg/common/service_details_codec_test.go | 2 +- pkg/common/types.go | 47 +++++++++++++------ pkg/common/types_test.go | 29 ++++++++++-- providers/aws/provider.go | 8 ++-- providers/aws/provider_test.go | 8 ++-- providers/aws/recommendations/client_test.go | 2 +- .../aws/recommendations/family_nu_test.go | 6 +-- providers/aws/recommendations/parser_sp.go | 8 ++-- .../parser_sp_additional_test.go | 4 +- providers/aws/services/savingsplans/client.go | 6 +-- .../aws/services/savingsplans/client_test.go | 38 +++++++-------- providers/azure/provider.go | 4 +- providers/azure/provider_test.go | 6 +-- .../azure/services/savingsplans/client.go | 4 +- .../services/savingsplans/client_test.go | 4 +- 24 files changed, 138 insertions(+), 98 deletions(-) diff --git a/cmd/helpers_test.go b/cmd/helpers_test.go index afbc0084d..6546035ae 100644 --- a/cmd/helpers_test.go +++ b/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{ diff --git a/cmd/main_test.go b/cmd/main_test.go index 79fdbd7b5..cf8ba4b3d 100644 --- a/cmd/main_test.go +++ b/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, }, { diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index 317fff5d6..9025bd18a 100644 --- a/cmd/multi_service_helpers.go +++ b/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", diff --git a/cmd/multi_service_stats.go b/cmd/multi_service_stats.go index 4ef161865..4c0f79314 100644 --- a/cmd/multi_service_stats.go +++ b/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 diff --git a/cmd/multi_service_stats_test.go b/cmd/multi_service_stats_test.go index 2af805ff5..d571ddd59 100644 --- a/cmd/multi_service_stats_test.go +++ b/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,7 +362,7 @@ 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", @@ -370,7 +370,7 @@ func TestPrintSavingsPlansSection(t *testing.T) { }, }, { - 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", diff --git a/cmd/multi_service_test.go b/cmd/multi_service_test.go index 7187e6232..d12c6f318 100644 --- a/cmd/multi_service_test.go +++ b/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{}, diff --git a/internal/purchase/coverage_extra_test.go b/internal/purchase/coverage_extra_test.go index 078145fc4..cf4768f35 100644 --- a/internal/purchase/coverage_extra_test.go +++ b/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}, diff --git a/internal/purchase/execution.go b/internal/purchase/execution.go index a5a43b890..030019901 100644 --- a/internal/purchase/execution.go +++ b/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, diff --git a/pkg/common/service_details_codec.go b/pkg/common/service_details_codec.go index 75a43f75e..482ce630f 100644 --- a/pkg/common/service_details_codec.go +++ b/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), diff --git a/pkg/common/service_details_codec_test.go b/pkg/common/service_details_codec_test.go index 37243fe51..5953786b7 100644 --- a/pkg/common/service_details_codec_test.go +++ b/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), diff --git a/pkg/common/types.go b/pkg/common/types.go index f6df840c3..ae623049c 100644 --- a/pkg/common/types.go +++ b/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 { diff --git a/pkg/common/types_test.go b/pkg/common/types_test.go index 70109d35f..1949a8b61 100644 --- a/pkg/common/types_test.go +++ b/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) { diff --git a/providers/aws/provider.go b/providers/aws/provider.go index c2b2d5a13..e9bbec0a8 100644 --- a/providers/aws/provider.go +++ b/providers/aws/provider.go @@ -433,9 +433,9 @@ func (p *AWSProvider) GetSupportedServices() []common.ServiceType { common.ServiceSavingsPlansEC2Instance, common.ServiceSavingsPlansSageMaker, common.ServiceSavingsPlansDatabase, - // Legacy umbrella for backward compat with persisted records; + // Umbrella sentinel for backward compat with persisted records; // see the GetServiceClient case below for rationale. - common.ServiceSavingsPlans, + common.ServiceSavingsPlansAll, // Legacy service types for backward compatibility common.ServiceEC2, common.ServiceRDS, @@ -475,8 +475,8 @@ func (p *AWSProvider) GetServiceClient(ctx context.Context, service common.Servi common.ServiceSavingsPlansDatabase: pt, _ := savingsplans.PlanTypeForServiceType(service) return NewSavingsPlansClient(regionalCfg, pt), nil - case common.ServiceSavingsPlans: - // Legacy umbrella path for any persisted RecommendationRecord + case common.ServiceSavingsPlansAll: + // Umbrella sentinel path for any persisted RecommendationRecord // (or scheduled purchase) still tagged with the pre-split slug. // The constructor's empty plan type signals "umbrella mode" to // the SP client: GetExistingCommitments returns every plan type diff --git a/providers/aws/provider_test.go b/providers/aws/provider_test.go index 650caabcd..8753b1506 100644 --- a/providers/aws/provider_test.go +++ b/providers/aws/provider_test.go @@ -251,8 +251,8 @@ func TestAWSProvider_GetSupportedServices(t *testing.T) { assert.Contains(t, services, common.ServiceSavingsPlansEC2Instance) assert.Contains(t, services, common.ServiceSavingsPlansSageMaker) assert.Contains(t, services, common.ServiceSavingsPlansDatabase) - // Legacy umbrella SP slug for backward compat with persisted records. - assert.Contains(t, services, common.ServiceSavingsPlans) + // Umbrella SP sentinel for backward compat with persisted records. + assert.Contains(t, services, common.ServiceSavingsPlansAll) // Legacy types assert.Contains(t, services, common.ServiceEC2) assert.Contains(t, services, common.ServiceRDS) @@ -323,10 +323,10 @@ func TestAWSProvider_GetServiceClient_AllServiceTypes(t *testing.T) { {common.ServiceSavingsPlansEC2Instance, common.ServiceSavingsPlansEC2Instance}, {common.ServiceSavingsPlansSageMaker, common.ServiceSavingsPlansSageMaker}, {common.ServiceSavingsPlansDatabase, common.ServiceSavingsPlansDatabase}, - // Legacy umbrella: GetServiceClient returns an SP client with + // Umbrella sentinel: GetServiceClient returns an SP client with // an empty plan-type filter (umbrella mode); GetServiceType // reports the umbrella slug, matching pre-split behaviour. - {common.ServiceSavingsPlans, common.ServiceSavingsPlans}, + {common.ServiceSavingsPlansAll, common.ServiceSavingsPlansAll}, } for _, tc := range testCases { diff --git a/providers/aws/recommendations/client_test.go b/providers/aws/recommendations/client_test.go index 63c45779c..a86557c1c 100644 --- a/providers/aws/recommendations/client_test.go +++ b/providers/aws/recommendations/client_test.go @@ -234,7 +234,7 @@ func TestGetRecommendations_SavingsPlans_Success(t *testing.T) { client := NewClientWithAPI(mockAPI, "us-east-1") params := common.RecommendationParams{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "partial-upfront", Term: "1yr", LookbackPeriod: "7d", diff --git a/providers/aws/recommendations/family_nu_test.go b/providers/aws/recommendations/family_nu_test.go index e24740698..ce4000db9 100644 --- a/providers/aws/recommendations/family_nu_test.go +++ b/providers/aws/recommendations/family_nu_test.go @@ -261,7 +261,7 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { // in the nonRDS slice for per-pool sizing. recs := []common.Recommendation{ {Service: common.ServiceEC2, CommitmentType: common.CommitmentReservedInstance, Count: 3}, - {Service: common.ServiceSavingsPlans, CommitmentType: common.CommitmentSavingsPlan, Count: 1}, + {Service: common.ServiceSavingsPlansAll, CommitmentType: common.CommitmentSavingsPlan, Count: 1}, { Service: common.ServiceRDS, CommitmentType: common.CommitmentReservedInstance, @@ -280,7 +280,7 @@ func TestApplyFamilyNUSizingRDS(t *testing.T) { require.Len(t, sized, 1, "only the RDS rec went through family-NU") require.Len(t, nonRDS, 2, "EC2 + SP recs left for per-pool sizing") assert.Equal(t, common.ServiceEC2, nonRDS[0].Service) - assert.Equal(t, common.ServiceSavingsPlans, nonRDS[1].Service) + assert.Equal(t, common.ServiceSavingsPlansAll, nonRDS[1].Service) }) t.Run("ProjectedCoverage is cumulative across recs in a family", func(t *testing.T) { @@ -379,7 +379,7 @@ func TestIsRDSRIRec(t *testing.T) { {"RelationalDB alias", common.Recommendation{Service: common.ServiceRelationalDB, CommitmentType: common.CommitmentReservedInstance}, true}, {"EC2 RI", common.Recommendation{Service: common.ServiceEC2, CommitmentType: common.CommitmentReservedInstance}, false}, {"RDS SP (impossible but defensive)", common.Recommendation{Service: common.ServiceRDS, CommitmentType: common.CommitmentSavingsPlan}, false}, - {"SP", common.Recommendation{Service: common.ServiceSavingsPlans, CommitmentType: common.CommitmentSavingsPlan}, false}, + {"SP", common.Recommendation{Service: common.ServiceSavingsPlansAll, CommitmentType: common.CommitmentSavingsPlan}, false}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/providers/aws/recommendations/parser_sp.go b/providers/aws/recommendations/parser_sp.go index 41b865eaf..9667fbadf 100644 --- a/providers/aws/recommendations/parser_sp.go +++ b/providers/aws/recommendations/parser_sp.go @@ -25,7 +25,7 @@ import ( // the per-plan-type split: each registered SP service makes its own // Cost Explorer call with its own term/payment defaults. // 2. Otherwise, fall back to the legacy IncludeSPTypes/ExcludeSPTypes -// filter mechanism (for callers passing the umbrella ServiceSavingsPlans +// filter mechanism (for callers passing the umbrella ServiceSavingsPlansAll // slug or for direct CLI invocations that haven't been migrated yet). func (c *Client) getSavingsPlansRecommendations(ctx context.Context, params common.RecommendationParams) ([]common.Recommendation, error) { planTypes := planTypesForParams(params) @@ -305,7 +305,7 @@ func planTypesForParams(params common.RecommendationParams) []types.SupportedSav // planTypeForServiceSlug maps a per-plan-type SP service slug to its // Cost Explorer plan-type enum. Returns false for non-SP slugs and for the -// legacy umbrella ServiceSavingsPlans (which still triggers the iterate-all +// umbrella sentinel ServiceSavingsPlansAll (which still triggers the iterate-all // fallback inside planTypesForParams). func planTypeForServiceSlug(s common.ServiceType) (types.SupportedSavingsPlansType, bool) { switch s { @@ -323,7 +323,7 @@ func planTypeForServiceSlug(s common.ServiceType) (types.SupportedSavingsPlansTy // serviceSlugForPlanType is the inverse of planTypeForServiceSlug. Used by // parseSavingsPlanDetail to tag each Recommendation with the per-plan-type -// slug rather than the legacy umbrella, so downstream stats/filters can +// slug rather than the umbrella sentinel, so downstream stats/filters can // distinguish Compute SP from SageMaker SP recommendations. func serviceSlugForPlanType(pt types.SupportedSavingsPlansType) common.ServiceType { switch pt { @@ -336,7 +336,7 @@ func serviceSlugForPlanType(pt types.SupportedSavingsPlansType) common.ServiceTy case types.SupportedSavingsPlansTypeDatabaseSp: return common.ServiceSavingsPlansDatabase } - return common.ServiceSavingsPlans + return common.ServiceSavingsPlansAll } // getFilteredPlanTypes returns the list of Savings Plan types to query based diff --git a/providers/aws/recommendations/parser_sp_additional_test.go b/providers/aws/recommendations/parser_sp_additional_test.go index cbe28a128..bd7637c43 100644 --- a/providers/aws/recommendations/parser_sp_additional_test.go +++ b/providers/aws/recommendations/parser_sp_additional_test.go @@ -277,7 +277,7 @@ func TestGetSavingsPlansRecommendations_WithFilters(t *testing.T) { client := NewClientWithAPI(mockAPI, "us-east-1") params := common.RecommendationParams{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "partial-upfront", Term: "1yr", LookbackPeriod: "7d", @@ -294,7 +294,7 @@ func TestGetSavingsPlansRecommendations_EmptyFilters(t *testing.T) { client := &Client{} params := common.RecommendationParams{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "partial-upfront", Term: "1yr", LookbackPeriod: "7d", diff --git a/providers/aws/services/savingsplans/client.go b/providers/aws/services/savingsplans/client.go index 4cea975ee..6e6a0096e 100644 --- a/providers/aws/services/savingsplans/client.go +++ b/providers/aws/services/savingsplans/client.go @@ -75,7 +75,7 @@ func ServiceTypeForPlanType(pt types.SavingsPlanType) common.ServiceType { case types.SavingsPlanTypeDatabase: return common.ServiceSavingsPlansDatabase } - return common.ServiceSavingsPlans + return common.ServiceSavingsPlansAll } // PlanTypeForServiceType is the inverse mapping: a common.ServiceType slug to @@ -117,8 +117,8 @@ func (c *Client) GetExistingCommitments(ctx context.Context) ([]common.Commitmen // The provider registers four SP services and calls GetExistingCommitments // on each; without filtering, every SP commitment would surface four times. // - // An empty planType signals legacy umbrella mode (the - // `case common.ServiceSavingsPlans` branch in provider.go's + // An empty planType signals umbrella sentinel mode (the + // `case common.ServiceSavingsPlansAll` branch in provider.go's // GetServiceClient): in that mode, return every commitment unfiltered // to match pre-split behaviour. Per-plan-type clients still partition. commitments := make([]common.Commitment, 0) diff --git a/providers/aws/services/savingsplans/client_test.go b/providers/aws/services/savingsplans/client_test.go index e52191a1e..de311a9f8 100644 --- a/providers/aws/services/savingsplans/client_test.go +++ b/providers/aws/services/savingsplans/client_test.go @@ -220,7 +220,7 @@ func TestClient_GetExistingCommitments(t *testing.T) { // TestClient_GetExistingCommitments_UmbrellaMode verifies that an SP client // constructed with an empty plan type (legacy umbrella mode, used by the -// AWS provider's `case common.ServiceSavingsPlans` branch in GetServiceClient) +// AWS provider's `case common.ServiceSavingsPlansAll` branch in GetServiceClient) // returns every commitment unfiltered — matching pre-issue-#22-split // behaviour for any persisted RecommendationRecord still tagged with the // umbrella slug. @@ -411,7 +411,7 @@ func TestClient_ValidateOffering(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", PaymentOption: "all-upfront", Term: "1yr", @@ -440,7 +440,7 @@ func TestClient_ValidateOffering_InvalidDetails(t *testing.T) { // Use ComputeDetails instead of SavingsPlanDetails to test type assertion failure rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, Details: common.ComputeDetails{ InstanceType: "t3.micro", }, @@ -459,7 +459,7 @@ func TestClient_PurchaseCommitment(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", Count: 1, PaymentOption: "all-upfront", @@ -503,7 +503,7 @@ func TestClient_PurchaseCommitment_SetsClientTokenForIdempotency(t *testing.T) { client := &Client{client: mockSP, region: "us-east-1"} rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", Count: 1, PaymentOption: "all-upfront", @@ -545,7 +545,7 @@ func TestClient_PurchaseCommitment_NoClientTokenWhenUnset(t *testing.T) { client := &Client{client: mockSP, region: "us-east-1"} rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", Count: 1, PaymentOption: "all-upfront", @@ -577,7 +577,7 @@ func TestClient_PurchaseCommitment_InvalidDetails(t *testing.T) { // Use ComputeDetails instead of SavingsPlanDetails to test type assertion failure rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, Details: common.ComputeDetails{ InstanceType: "t3.micro", }, @@ -598,7 +598,7 @@ func TestClient_PurchaseCommitment_OfferingNotFound(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -628,7 +628,7 @@ func TestClient_PurchaseCommitment_CreateFails(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -663,7 +663,7 @@ func TestClient_PurchaseCommitment_EmptyResponse(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -700,7 +700,7 @@ func TestClient_GetOfferingDetails(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", PaymentOption: "all-upfront", Term: "1yr", @@ -741,7 +741,7 @@ func TestClient_GetOfferingDetails_3YearTerm(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "EC2Instance", PaymentOption: "partial-upfront", Term: "3yr", @@ -781,7 +781,7 @@ func TestClient_GetOfferingDetails_NoUpfront(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, ResourceType: "Compute", PaymentOption: "no-upfront", Term: "1yr", @@ -819,7 +819,7 @@ func TestClient_GetOfferingDetails_InvalidDetails(t *testing.T) { // Use ComputeDetails instead of SavingsPlanDetails to test type assertion failure rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, Details: common.ComputeDetails{ InstanceType: "t3.micro", }, @@ -840,7 +840,7 @@ func TestClient_GetOfferingDetails_RatesError(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -890,7 +890,7 @@ func TestClient_FindOfferingID_AllPlanTypes(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -951,7 +951,7 @@ func TestClient_FindOfferingID_AllPaymentOptions(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: tt.paymentOption, Term: "1yr", Details: &common.SavingsPlanDetails{ @@ -1007,7 +1007,7 @@ func TestClient_FindOfferingID_TermVariations(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: tt.term, Details: &common.SavingsPlanDetails{ @@ -1045,7 +1045,7 @@ func TestClient_FindOfferingID_APIError(t *testing.T) { } rec := common.Recommendation{ - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, PaymentOption: "all-upfront", Term: "1yr", Details: &common.SavingsPlanDetails{ diff --git a/providers/azure/provider.go b/providers/azure/provider.go index 2817e7120..555c78145 100644 --- a/providers/azure/provider.go +++ b/providers/azure/provider.go @@ -432,7 +432,7 @@ func (p *AzureProvider) GetSupportedServices() []common.ServiceType { common.ServiceNoSQL, common.ServiceCache, common.ServiceMemoryDB, - common.ServiceSavingsPlans, + common.ServiceSavingsPlansAll, common.ServiceSearch, common.ServiceDataWarehouse, } @@ -490,7 +490,7 @@ func (p *AzureProvider) newServiceClientForSubscription(service common.ServiceTy return NewCosmosDBClient(p.cred, subscriptionID, region), nil case common.ServiceMemoryDB: return NewManagedRedisClient(p.cred, subscriptionID, region), nil - case common.ServiceSavingsPlans: + case common.ServiceSavingsPlansAll: return NewSavingsPlansClient(p.cred, subscriptionID, region), nil case common.ServiceSearch: return NewSearchClient(p.cred, subscriptionID, region), nil diff --git a/providers/azure/provider_test.go b/providers/azure/provider_test.go index 432c9d86c..0296264ef 100644 --- a/providers/azure/provider_test.go +++ b/providers/azure/provider_test.go @@ -265,7 +265,7 @@ func TestAzureProvider_GetSupportedServices(t *testing.T) { assert.Contains(t, services, common.ServiceNoSQL) assert.Contains(t, services, common.ServiceCache) assert.Contains(t, services, common.ServiceMemoryDB) - assert.Contains(t, services, common.ServiceSavingsPlans) + assert.Contains(t, services, common.ServiceSavingsPlansAll) assert.Contains(t, services, common.ServiceSearch) assert.Contains(t, services, common.ServiceDataWarehouse) } @@ -414,7 +414,7 @@ func TestAzureProvider_GetServiceClient_AllServiceTypes(t *testing.T) { {common.ServiceNoSQL}, {common.ServiceCache}, {common.ServiceMemoryDB}, - {common.ServiceSavingsPlans}, + {common.ServiceSavingsPlansAll}, {common.ServiceSearch}, {common.ServiceDataWarehouse}, } @@ -1162,7 +1162,7 @@ func TestAzureProvider_GetServiceClientForAccount(t *testing.T) { common.ServiceNoSQL, common.ServiceCache, common.ServiceMemoryDB, - common.ServiceSavingsPlans, + common.ServiceSavingsPlansAll, common.ServiceSearch, common.ServiceDataWarehouse, } diff --git a/providers/azure/services/savingsplans/client.go b/providers/azure/services/savingsplans/client.go index 8d98cf6b3..15db11bc5 100644 --- a/providers/azure/services/savingsplans/client.go +++ b/providers/azure/services/savingsplans/client.go @@ -112,7 +112,7 @@ func (c *Client) SetRPValidateClient(api RPValidateAPI) { // GetServiceType returns the service type for this client. func (c *Client) GetServiceType() common.ServiceType { - return common.ServiceSavingsPlans + return common.ServiceSavingsPlansAll } // GetRegion returns the region for this client. @@ -174,7 +174,7 @@ func convertSavingsPlan(sp *armbillingbenefits.SavingsPlanModel, subscriptionID Account: subscriptionID, CommitmentID: *sp.ID, CommitmentType: common.CommitmentSavingsPlan, - Service: common.ServiceSavingsPlans, + Service: common.ServiceSavingsPlansAll, Count: 1, State: "active", } diff --git a/providers/azure/services/savingsplans/client_test.go b/providers/azure/services/savingsplans/client_test.go index aa0b624e0..1ce975968 100644 --- a/providers/azure/services/savingsplans/client_test.go +++ b/providers/azure/services/savingsplans/client_test.go @@ -99,7 +99,7 @@ func TestNewClient(t *testing.T) { func TestGetServiceType(t *testing.T) { c := NewClient(nil, "sub", "eastus") - assert.Equal(t, common.ServiceSavingsPlans, c.GetServiceType()) + assert.Equal(t, common.ServiceSavingsPlansAll, c.GetServiceType()) } func TestGetRegion(t *testing.T) { @@ -174,7 +174,7 @@ func TestGetExistingCommitments_Happy(t *testing.T) { got := commitments[0] assert.Equal(t, common.ProviderAzure, got.Provider) assert.Equal(t, common.CommitmentSavingsPlan, got.CommitmentType) - assert.Equal(t, common.ServiceSavingsPlans, got.Service) + assert.Equal(t, common.ServiceSavingsPlansAll, got.Service) assert.Equal(t, spID, got.CommitmentID) assert.Equal(t, spName, got.ResourceType) assert.Equal(t, "Succeeded", got.State) From 50519984497e815f5629f6e28cda666948796e97 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 1 Jun 2026 19:20:01 +0200 Subject: [PATCH 2/3] fix(ci): resolve gofmt alignment in multi_service_helpers.go on PR #787 --- cmd/multi_service_helpers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/multi_service_helpers.go b/cmd/multi_service_helpers.go index 9025bd18a..3d9b56dbc 100644 --- a/cmd/multi_service_helpers.go +++ b/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.ServiceSavingsPlansAll: "Savings Plans", + common.ServiceSavingsPlansAll: "Savings Plans", common.ServiceSavingsPlansCompute: "Compute Savings Plans", common.ServiceSavingsPlansEC2Instance: "EC2 Instance Savings Plans", common.ServiceSavingsPlansSageMaker: "SageMaker Savings Plans", From 7ffd4c026a5d5dc2e5ac42f96f4d41442ebf408e Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Sun, 7 Jun 2026 14:45:37 -0700 Subject: [PATCH 3/3] fix(recommendations): update callers missed by ServiceSavingsPlans->All rename Rebase onto feat/multicloud-web-frontend brought in three files that still referenced the old common.ServiceSavingsPlans sentinel (renamed to ServiceSavingsPlansAll in the prior commit). Update the two production call sites and one test file so go vet and go build pass on the merged PR view. --- providers/aws/recommendations/client.go | 2 +- providers/azure/recommendations.go | 2 +- providers/azure/recommendations_test.go | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/providers/aws/recommendations/client.go b/providers/aws/recommendations/client.go index 01257deb5..8ebefa07a 100644 --- a/providers/aws/recommendations/client.go +++ b/providers/aws/recommendations/client.go @@ -344,7 +344,7 @@ func (c *Client) GetAllRecommendations(ctx context.Context) ([]common.Recommenda return nil }) g.Go(func() error { - spRecs, spErr = c.GetRecommendationsForService(gctx, common.ServiceSavingsPlans) + spRecs, spErr = c.GetRecommendationsForService(gctx, common.ServiceSavingsPlansAll) return nil }) diff --git a/providers/azure/recommendations.go b/providers/azure/recommendations.go index 8fe3719cd..560e524e6 100644 --- a/providers/azure/recommendations.go +++ b/providers/azure/recommendations.go @@ -134,7 +134,7 @@ func (r *RecommendationsClientAdapter) GetRecommendations(ctx context.Context, p // The call returns an empty slice so the service appears in the fan-out // and will start returning data once the API stabilises without requiring // a scheduler change. - if shouldIncludeService(params, common.ServiceSavingsPlans) { + if shouldIncludeService(params, common.ServiceSavingsPlansAll) { goService(&spErr, func() { spClient := savingsplans.NewClient(r.cred, r.subscriptionID, "") spRecs, spErr = spClient.GetRecommendations(gctx, params) diff --git a/providers/azure/recommendations_test.go b/providers/azure/recommendations_test.go index efdca18dc..833d3e2c4 100644 --- a/providers/azure/recommendations_test.go +++ b/providers/azure/recommendations_test.go @@ -218,8 +218,8 @@ func TestRecommendationsClientAdapter_GetAllRecommendations(t *testing.T) { } // TestGetRecommendations_SavingsPlansServiceIncluded pins that shouldIncludeService -// allows ServiceSavingsPlans through both when params.Service is empty (all-services -// sweep) and when explicitly set to ServiceSavingsPlans, and does not include it +// allows ServiceSavingsPlansAll through both when params.Service is empty (all-services +// sweep) and when explicitly set to ServiceSavingsPlansAll, and does not include it // when a different service is requested. This ensures the SP goroutine added to the // fan-out in GetRecommendations is exercised on every scheduler collection run. func TestGetRecommendations_SavingsPlansServiceIncluded(t *testing.T) { @@ -235,7 +235,7 @@ func TestGetRecommendations_SavingsPlansServiceIncluded(t *testing.T) { }, { name: "explicit savingsplans service is included", - params: common.RecommendationParams{Service: common.ServiceSavingsPlans}, + params: common.RecommendationParams{Service: common.ServiceSavingsPlansAll}, expected: true, }, { @@ -246,7 +246,7 @@ func TestGetRecommendations_SavingsPlansServiceIncluded(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := shouldIncludeService(tt.params, common.ServiceSavingsPlans) + got := shouldIncludeService(tt.params, common.ServiceSavingsPlansAll) assert.Equal(t, tt.expected, got) }) } @@ -431,7 +431,7 @@ func TestMergeServiceResults_OrderIsStable(t *testing.T) { dbRec := mkRec(common.ServiceRelationalDB, "database") cacheRec := mkRec(common.ServiceCache, "cache") cosmosRec := mkRec(common.ServiceNoSQL, "cosmosdb") - spRec := mkRec(common.ServiceSavingsPlans, "savingsplans") + spRec := mkRec(common.ServiceSavingsPlansAll, "savingsplans") advisorRec := mkRec(common.ServiceCompute, "advisor") // Advisor produces Compute recs // Replicate the exact call order from GetRecommendations.