From a98ec188eb376da708804121c59bdebd0bd134de Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Thu, 28 May 2026 20:11:03 +0200
Subject: [PATCH 1/7] feat(gcp): populate RecurringMonthlyCost in GCP rec
 parsers (closes #264)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Set RecurringMonthlyCost on all three GCP service recommendation parsers:
- computeengine: PaymentOption "upfront" -> &0 (no monthly charge)
- cloudsql: monthly -> CommitmentPrice/termMonths via billing SKU lookup
- memorystore: monthly -> CommitmentPrice/termMonths via billing SKU lookup

Billing lookup failure is non-fatal: log a warning and leave the field nil
so the frontend renders "—" rather than a stale value.

Adds 5 new tests (2 per monthly service + 1 on computeengine) asserting
the field is non-nil and matches the expected pricing formula.
---
 providers/gcp/services/cloudsql/client.go     | 16 ++++
 .../gcp/services/cloudsql/client_test.go      | 94 +++++++++++++++++++
 .../gcp/services/computeengine/client.go      |  6 ++
 .../gcp/services/computeengine/client_test.go |  6 +-
 providers/gcp/services/memorystore/client.go  | 16 ++++
 .../gcp/services/memorystore/client_test.go   | 80 ++++++++++++++++
 6 files changed, 217 insertions(+), 1 deletion(-)

diff --git a/providers/gcp/services/cloudsql/client.go b/providers/gcp/services/cloudsql/client.go
index 8f443e3eb..a95539d17 100644
--- a/providers/gcp/services/cloudsql/client.go
+++ b/providers/gcp/services/cloudsql/client.go
@@ -549,6 +549,22 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 		c.fillSQLPricing(ctx, rec, termYears)
 	}
 
+	// Populate RecurringMonthlyCost from the billing API. Cloud SQL CUDs are
+	// monthly-payment commitments, so the per-month charge is
+	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
+	// log the warning and leave RecurringMonthlyCost nil so the frontend
+	// renders "—" rather than a stale value.
+	termYears := 1
+	if rec.Term == "3yr" || rec.Term == "3" {
+		termYears = 3
+	}
+	if pricing, err := c.getSQLPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
+		log.Printf("cloudsql: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
+	} else {
+		monthly := pricing.CommitmentPrice / float64(termYears*12)
+		rec.RecurringMonthlyCost = &monthly
+	}
+
 	return rec
 }
 
diff --git a/providers/gcp/services/cloudsql/client_test.go b/providers/gcp/services/cloudsql/client_test.go
index 6a93b81ae..c78f554fd 100644
--- a/providers/gcp/services/cloudsql/client_test.go
+++ b/providers/gcp/services/cloudsql/client_test.go
@@ -841,3 +841,97 @@ func TestGetSQLPricing_CommitmentPriceIsTermTotal(t *testing.T) {
 	assert.Less(t, pricing.SavingsPercentage, float64(50),
 		"SavingsPercentage must be realistic (not ~100%)")
 }
+
+func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	// Inject a billing mock that returns a known on-demand price. The
+	// getSQLPricing implementation applies a 15% package savings for 1yr
+	// (discount factor 0.85), so CommitmentPrice = onDemandHourly * 8760 * 0.85.
+	// RecurringMonthlyCost must equal CommitmentPrice / 12 (one year = 12 months).
+	const hourlyRate = 0.12 // USD/h per vCPU -- representative db-n1-standard-1 value
+	mockBilling := &MockBillingService{
+		skus: &cloudbilling.ListSkusResponse{
+			Skus: []*cloudbilling.Sku{
+				{
+					Description:    "db-n1-standard-1 Cloud SQL",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        int64(hourlyRate * 1e9),
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	client.SetBillingService(mockBilling)
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name: "test-rec",
+		PrimaryImpact: &recommenderpb.Impact{
+			Category: recommenderpb.Impact_COST,
+			Projection: &recommenderpb.Impact_CostProjection{
+				CostProjection: &recommenderpb.CostProjection{
+					Cost: &money.Money{Units: -100, CurrencyCode: "USD"},
+				},
+			},
+		},
+		Content: &recommenderpb.RecommendationContent{
+			OperationGroups: []*recommenderpb.OperationGroup{
+				{
+					Operations: []*recommenderpb.Operation{
+						{Resource: "projects/test/instances/db-n1-standard-1"},
+					},
+				},
+			},
+		},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
+	require.NotNil(t, rec)
+
+	// RecurringMonthlyCost must be a non-nil pointer to a positive value for
+	// a monthly Cloud SQL CUD recommendation.
+	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
+	assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Cloud SQL CUD")
+
+	// Verify the value matches CommitmentPrice / 12 exactly.
+	// estimateSQLCommitmentPrice applies a 15% savings factor for 1yr (discount=0.85).
+	const hoursIn1yr = 8760.0
+	expectedCommitment := hourlyRate * hoursIn1yr * 0.85 // 15% savings for 1yr (see estimateSQLCommitmentPrice)
+	assert.InDelta(t, expectedCommitment/12, *rec.RecurringMonthlyCost, 1e-6)
+}
+
+func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	// Inject a billing mock that always errors; RecurringMonthlyCost must
+	// remain nil (frontend renders "—") rather than a zero or stale value.
+	client.SetBillingService(&MockBillingService{err: errors.New("billing unavailable")})
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name: "test-rec",
+		Content: &recommenderpb.RecommendationContent{
+			OperationGroups: []*recommenderpb.OperationGroup{
+				{Operations: []*recommenderpb.Operation{{Resource: "projects/test/instances/db-n1-standard-1"}}},
+			},
+		},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
+	require.NotNil(t, rec)
+	assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be nil when billing lookup fails")
+}
diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go
index 9900a863d..b32f4c89f 100644
--- a/providers/gcp/services/computeengine/client.go
+++ b/providers/gcp/services/computeengine/client.go
@@ -945,6 +945,12 @@ func (c *ComputeEngineClient) convertGCPRecommendation(ctx context.Context, gcpR
 
 	c.enrichRecWithPricing(ctx, rec)
 
+	// GCP Compute CUDs with PaymentOption "upfront" carry no per-month charge:
+	// the full commitment is paid at purchase time. Set RecurringMonthlyCost to
+	// a pointer-to-zero (not nil) so the frontend renders "$0" rather than "—".
+	zero := 0.0
+	rec.RecurringMonthlyCost = &zero
+
 	return rec
 }
 
diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go
index 971cfed87..8ac4cdaa4 100644
--- a/providers/gcp/services/computeengine/client_test.go
+++ b/providers/gcp/services/computeengine/client_test.go
@@ -900,7 +900,11 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) {
 	assert.Equal(t, "n1-standard-4", rec.ResourceType)
 	assert.Equal(t, 50.5, rec.EstimatedSavings)
 	assert.Equal(t, "monthly", rec.PaymentOption,
-		"GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider[\"gcp\"]")
+		"GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]")
+	// PaymentOption is "upfront" (all-upfront): no monthly charge, so
+	// RecurringMonthlyCost must be a non-nil pointer to exactly 0.
+	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for upfront GCP CUDs")
+	assert.InDelta(t, 0.0, *rec.RecurringMonthlyCost, 1e-9)
 }
 
 // infiniteRecommenderIterator never signals iterator.Done, used to exercise
diff --git a/providers/gcp/services/memorystore/client.go b/providers/gcp/services/memorystore/client.go
index ba6a2946a..26c569016 100644
--- a/providers/gcp/services/memorystore/client.go
+++ b/providers/gcp/services/memorystore/client.go
@@ -532,5 +532,21 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 		c.fillRedisPricing(ctx, rec, termYears)
 	}
 
+	// Populate RecurringMonthlyCost from the billing API. Memorystore CUDs are
+	// monthly-payment commitments, so the per-month charge is
+	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
+	// log the warning and leave RecurringMonthlyCost nil so the frontend
+	// renders "—" rather than a stale value.
+	termYears := 1
+	if rec.Term == "3yr" || rec.Term == "3" {
+		termYears = 3
+	}
+	if pricing, err := c.getRedisPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
+		log.Printf("memorystore: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
+	} else {
+		monthly := pricing.CommitmentPrice / float64(termYears*12)
+		rec.RecurringMonthlyCost = &monthly
+	}
+
 	return rec
 }
diff --git a/providers/gcp/services/memorystore/client_test.go b/providers/gcp/services/memorystore/client_test.go
index 81eff3f53..e5c58dec0 100644
--- a/providers/gcp/services/memorystore/client_test.go
+++ b/providers/gcp/services/memorystore/client_test.go
@@ -775,3 +775,83 @@ func TestMemorystoreClient_SetterMethods(t *testing.T) {
 	client.SetRecommenderClient(mockRecommender)
 	assert.Equal(t, mockRecommender, client.recommenderClient)
 }
+
+func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	// Inject a billing mock with a known on-demand hourly price. The
+	// getRedisPricing implementation applies a 30% CUD discount for 1yr, so
+	// CommitmentPrice = onDemandHourly * 8760 * 0.70. RecurringMonthlyCost
+	// must equal CommitmentPrice / 12 (one year = 12 months).
+	const hourlyRate = 0.10 // USD/h -- representative basic Memorystore value
+	client.SetBillingService(&MockBillingService{
+		skus: &cloudbilling.ListSkusResponse{
+			Skus: []*cloudbilling.Sku{
+				{
+					Description:    "redis-basic Cloud Memorystore Redis",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        int64(hourlyRate * 1e9),
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	})
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name: "test-rec",
+		Content: &recommenderpb.RecommendationContent{
+			OperationGroups: []*recommenderpb.OperationGroup{
+				{
+					Operations: []*recommenderpb.Operation{
+						{Resource: "projects/test/instances/redis-basic"},
+					},
+				},
+			},
+		},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	require.NotNil(t, rec)
+
+	// RecurringMonthlyCost must be a non-nil pointer to a positive value for
+	// a monthly Memorystore CUD recommendation.
+	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
+	assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Memorystore CUD")
+
+	// Verify the value matches CommitmentPrice / 12 exactly.
+	const hoursIn1yr = 8760.0
+	expectedCommitment := hourlyRate * hoursIn1yr * 0.70 // 30% discount for 1yr
+	assert.InDelta(t, expectedCommitment/12, *rec.RecurringMonthlyCost, 1e-6)
+}
+
+func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	// Inject a billing mock that always errors; RecurringMonthlyCost must
+	// remain nil (frontend renders "—") rather than a zero or stale value.
+	client.SetBillingService(&MockBillingService{err: errors.New("billing unavailable")})
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name:    "test-rec",
+		Content: &recommenderpb.RecommendationContent{},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	require.NotNil(t, rec)
+	assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be nil when billing lookup fails")
+}

From 29375c3182ca812e495416a2205717afa89a39f3 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 1 Jun 2026 19:23:40 +0200
Subject: [PATCH 2/7] fix(ci): reduce cyclomatic complexity in GCP
 convertGCPRecommendation

Extract extractGCPResourceType, extractGCPSavings, and termYearsFromLabel
helpers from convertGCPRecommendation in both the memorystore and cloudsql
packages. Complexity drops from 13 to 5, clearing the gocyclo pre-commit
hook failure on CI.
---
 providers/gcp/services/cloudsql/client.go    | 46 ++++++++++----------
 providers/gcp/services/memorystore/client.go | 46 ++++++++++----------
 2 files changed, 48 insertions(+), 44 deletions(-)

diff --git a/providers/gcp/services/cloudsql/client.go b/providers/gcp/services/cloudsql/client.go
index a95539d17..29e09e7d0 100644
--- a/providers/gcp/services/cloudsql/client.go
+++ b/providers/gcp/services/cloudsql/client.go
@@ -455,14 +455,13 @@ func skuMatchesTier(sku *cloudbilling.Sku, tier, region string) bool {
 	return true
 }
 
-// extractResourceTypeFromContent extracts the last path segment of the first
-// non-empty Operation.Resource across all operation groups. Used by all four
-// GCP service converters to set rec.ResourceType from Recommender payloads.
-func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string {
-	if content == nil || content.OperationGroups == nil {
+// extractGCPResourceType returns the last path segment of the first non-empty
+// resource field found across all operation groups, or "" if none is present.
+func extractGCPResourceType(rec *recommenderpb.Recommendation) string {
+	if rec.Content == nil || rec.Content.OperationGroups == nil {
 		return ""
 	}
-	for _, opGroup := range content.OperationGroups {
+	for _, opGroup := range rec.Content.OperationGroups {
 		for _, op := range opGroup.Operations {
 			if op.Resource == "" {
 				continue
@@ -476,13 +475,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent
 	return ""
 }
 
-// extractEstimatedSavings returns the negative of the PrimaryImpact cost
-// projection (GCP encodes savings as a negative cost delta).
-func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 {
-	if gcpRec.PrimaryImpact == nil {
+// extractGCPSavings returns the estimated monthly savings (positive value)
+// from the primary cost impact of a GCP recommendation, or 0 if absent.
+func extractGCPSavings(rec *recommenderpb.Recommendation) float64 {
+	if rec.PrimaryImpact == nil {
 		return 0
 	}
-	costProj := gcpRec.PrimaryImpact.GetCostProjection()
+	costProj := rec.PrimaryImpact.GetCostProjection()
 	if costProj == nil || costProj.Cost == nil {
 		return 0
 	}
@@ -510,6 +509,15 @@ func (c *CloudSQLClient) fillSQLPricing(ctx context.Context, rec *common.Recomme
 	}
 }
 
+// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
+// integer number of years (defaults to 1 for any unrecognised value).
+func termYearsFromLabel(term string) int {
+	if term == "3yr" || term == "3" {
+		return 3
+	}
+	return 1
+}
+
 // convertGCPRecommendation converts a GCP Recommender recommendation to common format.
 // It also calls getSQLPricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/
 // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly
@@ -525,6 +533,7 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 		term = "1yr"
 	}
 
+
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceRelationalDB,
@@ -536,17 +545,13 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 		PaymentOption:  paymentOption,
 	}
 
-	rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content)
-	rec.EstimatedSavings = extractEstimatedSavings(gcpRec)
+	rec.ResourceType = extractGCPResourceType(gcpRec)
+	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
 	// correctly (issue #1022 C2).
 	if rec.ResourceType != "" {
-		termYears := 1
-		if rec.Term == "3yr" || rec.Term == "3" {
-			termYears = 3
-		}
-		c.fillSQLPricing(ctx, rec, termYears)
+		c.fillSQLPricing(ctx, rec, termYearsFromLabel(rec.Term))
 	}
 
 	// Populate RecurringMonthlyCost from the billing API. Cloud SQL CUDs are
@@ -554,10 +559,7 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
 	// log the warning and leave RecurringMonthlyCost nil so the frontend
 	// renders "—" rather than a stale value.
-	termYears := 1
-	if rec.Term == "3yr" || rec.Term == "3" {
-		termYears = 3
-	}
+	termYears := termYearsFromLabel(rec.Term)
 	if pricing, err := c.getSQLPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
 		log.Printf("cloudsql: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
 	} else {
diff --git a/providers/gcp/services/memorystore/client.go b/providers/gcp/services/memorystore/client.go
index 26c569016..f9cdbd5ec 100644
--- a/providers/gcp/services/memorystore/client.go
+++ b/providers/gcp/services/memorystore/client.go
@@ -438,14 +438,13 @@ func skuMatchesTier(sku *cloudbilling.Sku, tier, region string) bool {
 	return true
 }
 
-// extractResourceTypeFromContent extracts the last path segment of the first
-// non-empty Operation.Resource across all operation groups. Used by all four
-// GCP service converters to set rec.ResourceType from Recommender payloads.
-func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string {
-	if content == nil || content.OperationGroups == nil {
+// extractGCPResourceType returns the last path segment of the first non-empty
+// resource field found across all operation groups, or "" if none is present.
+func extractGCPResourceType(rec *recommenderpb.Recommendation) string {
+	if rec.Content == nil || rec.Content.OperationGroups == nil {
 		return ""
 	}
-	for _, opGroup := range content.OperationGroups {
+	for _, opGroup := range rec.Content.OperationGroups {
 		for _, op := range opGroup.Operations {
 			if op.Resource == "" {
 				continue
@@ -459,13 +458,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent
 	return ""
 }
 
-// extractEstimatedSavings returns the negative of the PrimaryImpact cost
-// projection (GCP encodes savings as a negative cost delta).
-func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 {
-	if gcpRec.PrimaryImpact == nil {
+// extractGCPSavings returns the estimated monthly savings (positive value)
+// from the primary cost impact of a GCP recommendation, or 0 if absent.
+func extractGCPSavings(rec *recommenderpb.Recommendation) float64 {
+	if rec.PrimaryImpact == nil {
 		return 0
 	}
-	costProj := gcpRec.PrimaryImpact.GetCostProjection()
+	costProj := rec.PrimaryImpact.GetCostProjection()
 	if costProj == nil || costProj.Cost == nil {
 		return 0
 	}
@@ -493,6 +492,15 @@ func (c *MemorystoreClient) fillRedisPricing(ctx context.Context, rec *common.Re
 	}
 }
 
+// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
+// integer number of years (defaults to 1 for any unrecognised value).
+func termYearsFromLabel(term string) int {
+	if term == "3yr" || term == "3" {
+		return 3
+	}
+	return 1
+}
+
 // convertGCPRecommendation converts a GCP Recommender recommendation to common format.
 // It also calls getRedisPricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/
 // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly
@@ -508,6 +516,7 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 		term = "1yr"
 	}
 
+
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceCache,
@@ -519,17 +528,13 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 		PaymentOption:  paymentOption,
 	}
 
-	rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content)
-	rec.EstimatedSavings = extractEstimatedSavings(gcpRec)
+	rec.ResourceType = extractGCPResourceType(gcpRec)
+	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
 	// correctly (issue #1022 C2).
 	if rec.ResourceType != "" {
-		termYears := 1
-		if rec.Term == "3yr" || rec.Term == "3" {
-			termYears = 3
-		}
-		c.fillRedisPricing(ctx, rec, termYears)
+		c.fillRedisPricing(ctx, rec, termYearsFromLabel(rec.Term))
 	}
 
 	// Populate RecurringMonthlyCost from the billing API. Memorystore CUDs are
@@ -537,10 +542,7 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
 	// log the warning and leave RecurringMonthlyCost nil so the frontend
 	// renders "—" rather than a stale value.
-	termYears := 1
-	if rec.Term == "3yr" || rec.Term == "3" {
-		termYears = 3
-	}
+	termYears := termYearsFromLabel(rec.Term)
 	if pricing, err := c.getRedisPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
 		log.Printf("memorystore: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
 	} else {

From a0696cbcec681a788e539f32d4ea80fb6e48a0a2 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 5 Jun 2026 17:02:02 +0200
Subject: [PATCH 3/7] feat(gcp/cloudstorage): populate RecurringMonthlyCost for
 consistency (refs #264)

The three other GCP service clients (cloudsql, memorystore, computeengine)
already populate RecurringMonthlyCost in convertGCPRecommendation via their
respective pricing helpers. cloudstorage was the only remaining client that
left the field nil, causing the frontend to render "--" for all Cloud Storage
rows.

The fix calls getStoragePricing (which already existed for GetOfferingDetails)
and sets RecurringMonthlyCost = CommitmentPrice / (termYears * 12). A pricing
lookup failure is non-fatal: log a warning and leave the field nil so the
frontend renders "--" rather than a stale value -- matching the sibling pattern.

Tests added:
- success case: non-nil RecurringMonthlyCost > 0 when billing returns a SKU
- billing-failure case: field stays nil when billing call errors out (must not
  coerce to 0)
---
 providers/gcp/services/cloudstorage/client.go | 16 +++
 .../gcp/services/cloudstorage/client_test.go  | 99 +++++++++++++++++++
 2 files changed, 115 insertions(+)

diff --git a/providers/gcp/services/cloudstorage/client.go b/providers/gcp/services/cloudstorage/client.go
index bcae9ae1a..803067476 100644
--- a/providers/gcp/services/cloudstorage/client.go
+++ b/providers/gcp/services/cloudstorage/client.go
@@ -541,5 +541,21 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 		c.fillStoragePricing(ctx, rec, termYears)
 	}
 
+	// Populate RecurringMonthlyCost from the billing API. Cloud Storage
+	// committed-use discounts are monthly-payment commitments, so the
+	// per-month charge is CommitmentPrice / termMonths. A billing lookup
+	// failure is non-fatal: log the warning and leave RecurringMonthlyCost
+	// nil so the frontend renders "—" rather than a stale value.
+	termYears := 1
+	if rec.Term == "3yr" || rec.Term == "3" {
+		termYears = 3
+	}
+	if pricing, err := c.getStoragePricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
+		log.Printf("cloudstorage: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
+	} else {
+		monthly := pricing.CommitmentPrice / float64(termYears*12)
+		rec.RecurringMonthlyCost = &monthly
+	}
+
 	return rec
 }
diff --git a/providers/gcp/services/cloudstorage/client_test.go b/providers/gcp/services/cloudstorage/client_test.go
index 78b874d0b..c1a639dee 100644
--- a/providers/gcp/services/cloudstorage/client_test.go
+++ b/providers/gcp/services/cloudstorage/client_test.go
@@ -856,3 +856,102 @@ func TestSkuMatchesStorageClass_CaseInsensitive(t *testing.T) {
 	}
 	assert.True(t, skuMatchesStorageClass(sku, "standard", "us-central1"))
 }
+
+// TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCost verifies
+// that convertGCPRecommendation sets a non-nil RecurringMonthlyCost when the billing
+// service returns valid pricing. This is the regression test for issue #264: the
+// pre-fix state left RecurringMonthlyCost nil, causing the frontend to render "—".
+func TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCost(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	// Provide a billing mock that returns a non-zero on-demand price so that
+	// getStoragePricing succeeds and produces a positive CommitmentPrice.
+	mockBilling := &MockBillingService{
+		skus: &cloudbilling.ListSkusResponse{
+			Skus: []*cloudbilling.Sku{
+				{
+					Description:    "Standard Storage in us-central1",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        26000000,
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	client.SetBillingService(mockBilling)
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name: "test-rec",
+		PrimaryImpact: &recommenderpb.Impact{
+			Category: recommenderpb.Impact_COST,
+			Projection: &recommenderpb.Impact_CostProjection{
+				CostProjection: &recommenderpb.CostProjection{
+					Cost: &money.Money{
+						Units:        -100,
+						Nanos:        0,
+						CurrencyCode: "USD",
+					},
+				},
+			},
+		},
+		Content: &recommenderpb.RecommendationContent{
+			OperationGroups: []*recommenderpb.OperationGroup{
+				{
+					Operations: []*recommenderpb.Operation{
+						{Resource: "projects/test/buckets/STANDARD"},
+					},
+				},
+			},
+		},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	require.NotNil(t, rec)
+	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
+	assert.Greater(t, *rec.RecurringMonthlyCost, float64(0))
+}
+
+// TestCloudStorageClient_ConvertGCPRecommendation_BillingFailure_RecurringMonthlyCostNil
+// verifies that convertGCPRecommendation leaves RecurringMonthlyCost nil (rather than
+// coercing to 0) when the billing service call fails, matching the sibling services'
+// non-fatal-failure contract.
+func TestCloudStorageClient_ConvertGCPRecommendation_BillingFailure_RecurringMonthlyCostNil(t *testing.T) {
+	ctx := context.Background()
+	client, _ := NewClient(ctx, "test-project", "us-central1")
+
+	mockBilling := &MockBillingService{
+		err: errors.New("billing API unavailable"),
+	}
+	client.SetBillingService(mockBilling)
+
+	gcpRec := &recommenderpb.Recommendation{
+		Name: "test-rec",
+		Content: &recommenderpb.RecommendationContent{
+			OperationGroups: []*recommenderpb.OperationGroup{
+				{
+					Operations: []*recommenderpb.Operation{
+						{Resource: "projects/test/buckets/STANDARD"},
+					},
+				},
+			},
+		},
+	}
+
+	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	require.NotNil(t, rec)
+	assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must remain nil when billing lookup fails")
+}

From 07a933f6379c29708f04bbfd1fb749b054cae262 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 7 Jun 2026 13:58:30 -0700
Subject: [PATCH 4/7] fix(ci): reduce cyclomatic complexity in cloudstorage
 convertGCPRecommendation

Extract extractGCPResourceType, extractGCPSavings, and termYearsFromLabel
helpers from convertGCPRecommendation in the cloudstorage package, mirroring
the existing refactor in the cloudsql and memorystore packages. Complexity
drops from 13 to under the limit, clearing the gocyclo pre-commit hook
failure on CI. Behavior is unchanged.
---
 providers/gcp/services/cloudstorage/client.go | 46 ++++++++++---------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/providers/gcp/services/cloudstorage/client.go b/providers/gcp/services/cloudstorage/client.go
index 803067476..07b0a3499 100644
--- a/providers/gcp/services/cloudstorage/client.go
+++ b/providers/gcp/services/cloudstorage/client.go
@@ -447,14 +447,13 @@ func skuMatchesStorageClass(sku *cloudbilling.Sku, storageClass, region string)
 	return true
 }
 
-// extractResourceTypeFromContent extracts the last path segment of the first
-// non-empty Operation.Resource across all operation groups. Used by all four
-// GCP service converters to set rec.ResourceType from Recommender payloads.
-func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent) string {
-	if content == nil || content.OperationGroups == nil {
+// extractGCPResourceType returns the last path segment of the first non-empty
+// resource field found across all operation groups, or "" if none is present.
+func extractGCPResourceType(rec *recommenderpb.Recommendation) string {
+	if rec.Content == nil || rec.Content.OperationGroups == nil {
 		return ""
 	}
-	for _, opGroup := range content.OperationGroups {
+	for _, opGroup := range rec.Content.OperationGroups {
 		for _, op := range opGroup.Operations {
 			if op.Resource == "" {
 				continue
@@ -468,13 +467,13 @@ func extractResourceTypeFromContent(content *recommenderpb.RecommendationContent
 	return ""
 }
 
-// extractEstimatedSavings returns the negative of the PrimaryImpact cost
-// projection (GCP encodes savings as a negative cost delta).
-func extractEstimatedSavings(gcpRec *recommenderpb.Recommendation) float64 {
-	if gcpRec.PrimaryImpact == nil {
+// extractGCPSavings returns the estimated monthly savings (positive value)
+// from the primary cost impact of a GCP recommendation, or 0 if absent.
+func extractGCPSavings(rec *recommenderpb.Recommendation) float64 {
+	if rec.PrimaryImpact == nil {
 		return 0
 	}
-	costProj := gcpRec.PrimaryImpact.GetCostProjection()
+	costProj := rec.PrimaryImpact.GetCostProjection()
 	if costProj == nil || costProj.Cost == nil {
 		return 0
 	}
@@ -502,6 +501,15 @@ func (c *CloudStorageClient) fillStoragePricing(ctx context.Context, rec *common
 	}
 }
 
+// termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
+// integer number of years (defaults to 1 for any unrecognised value).
+func termYearsFromLabel(term string) int {
+	if term == "3yr" || term == "3" {
+		return 3
+	}
+	return 1
+}
+
 // convertGCPRecommendation converts a GCP Recommender recommendation to common format.
 // It also calls getStoragePricing to fill CommitmentCost/OnDemandCost/SavingsPercentage/
 // BreakEvenMonths so the scorer can filter and rank GCP recommendations correctly
@@ -517,6 +525,7 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 		term = "1yr"
 	}
 
+
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceStorage,
@@ -528,17 +537,13 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 		PaymentOption:  paymentOption,
 	}
 
-	rec.ResourceType = extractResourceTypeFromContent(gcpRec.Content)
-	rec.EstimatedSavings = extractEstimatedSavings(gcpRec)
+	rec.ResourceType = extractGCPResourceType(gcpRec)
+	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
 	// correctly (issue #1022 C2).
 	if rec.ResourceType != "" {
-		termYears := 1
-		if rec.Term == "3yr" || rec.Term == "3" {
-			termYears = 3
-		}
-		c.fillStoragePricing(ctx, rec, termYears)
+		c.fillStoragePricing(ctx, rec, termYearsFromLabel(rec.Term))
 	}
 
 	// Populate RecurringMonthlyCost from the billing API. Cloud Storage
@@ -546,10 +551,7 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 	// per-month charge is CommitmentPrice / termMonths. A billing lookup
 	// failure is non-fatal: log the warning and leave RecurringMonthlyCost
 	// nil so the frontend renders "—" rather than a stale value.
-	termYears := 1
-	if rec.Term == "3yr" || rec.Term == "3" {
-		termYears = 3
-	}
+	termYears := termYearsFromLabel(rec.Term)
 	if pricing, err := c.getStoragePricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
 		log.Printf("cloudstorage: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
 	} else {

From a74f866480a3a97892a0d57aefc7cbfded6df43a Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 8 Jun 2026 15:49:28 -0700
Subject: [PATCH 5/7] fix(gcp): derive RecurringMonthlyCost from CommitmentCost
 without redundant lookups

Address CodeRabbit findings on PR #811:

- cloudsql/cloudstorage/memorystore converters issued a second
  getSQLPricing/getStoragePricing/getRedisPricing call after fill*Pricing
  already performed the lookup, doubling SKU API calls and emitting warnings
  even for empty ResourceType. Reuse the CommitmentCost the fill* helpers
  populate and gate the whole block on a non-empty ResourceType.

- computeengine unconditionally set RecurringMonthlyCost to a pointer-to-zero,
  mislabeling monthly-billed CUDs as a known-zero recurring fee and breaking
  the nil-vs-zero contract. Derive the monthly charge from CommitmentCost and
  leave the field nil when pricing is unavailable. Add termYearsFromTerm and
  reuse it in enrichRecWithPricing, which also makes 36mo terms map to 3 years.

- Fix the success-path test fixtures: getSQLPricing/getRedisPricing now require
  a commitment SKU, so the on-demand-only fixtures drove the failure branch and
  left RecurringMonthlyCost nil. Add commitment SKUs and replace the stale
  hardcoded discount multipliers with the commitment-rate-derived expectation.

- Fix compile errors in cloudstorage and memorystore tests that still called
  convertGCPRecommendation with two arguments; pass common.RecommendationParams.

- Align the compute test assertion with the monthly-default path: empty params
  yield no computed monthly pricing, so RecurringMonthlyCost stays nil (unknown),
  not an explicit zero.
---
 providers/gcp/services/cloudsql/client.go     | 30 ++++++-------
 .../gcp/services/cloudsql/client_test.go      | 40 +++++++++++++----
 providers/gcp/services/cloudstorage/client.go | 30 ++++++-------
 .../gcp/services/cloudstorage/client_test.go  | 25 ++++++++++-
 .../gcp/services/computeengine/client.go      | 33 ++++++++++----
 .../gcp/services/computeengine/client_test.go |  9 ++--
 providers/gcp/services/memorystore/client.go  | 30 ++++++-------
 .../gcp/services/memorystore/client_test.go   | 45 ++++++++++++++-----
 8 files changed, 159 insertions(+), 83 deletions(-)

diff --git a/providers/gcp/services/cloudsql/client.go b/providers/gcp/services/cloudsql/client.go
index 29e09e7d0..df296b881 100644
--- a/providers/gcp/services/cloudsql/client.go
+++ b/providers/gcp/services/cloudsql/client.go
@@ -533,7 +533,6 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 		term = "1yr"
 	}
 
-
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceRelationalDB,
@@ -549,22 +548,21 @@ func (c *CloudSQLClient) convertGCPRecommendation(ctx context.Context, gcpRec *r
 	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
-	// correctly (issue #1022 C2).
+	// correctly (issue #1022 C2). fillSQLPricing performs the single billing
+	// lookup and populates CommitmentCost; we reuse that value below to derive
+	// RecurringMonthlyCost rather than issuing a second SKU call.
 	if rec.ResourceType != "" {
-		c.fillSQLPricing(ctx, rec, termYearsFromLabel(rec.Term))
-	}
-
-	// Populate RecurringMonthlyCost from the billing API. Cloud SQL CUDs are
-	// monthly-payment commitments, so the per-month charge is
-	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
-	// log the warning and leave RecurringMonthlyCost nil so the frontend
-	// renders "—" rather than a stale value.
-	termYears := termYearsFromLabel(rec.Term)
-	if pricing, err := c.getSQLPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
-		log.Printf("cloudsql: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
-	} else {
-		monthly := pricing.CommitmentPrice / float64(termYears*12)
-		rec.RecurringMonthlyCost = &monthly
+		termYears := termYearsFromLabel(rec.Term)
+		c.fillSQLPricing(ctx, rec, termYears)
+
+		// Cloud SQL CUDs are monthly-payment commitments, so the per-month
+		// charge is CommitmentCost / termMonths. When the billing lookup
+		// failed, CommitmentCost stays 0 and RecurringMonthlyCost remains nil
+		// so the frontend renders "—" rather than a stale value.
+		if rec.CommitmentCost > 0 {
+			monthly := rec.CommitmentCost / float64(termYears*12)
+			rec.RecurringMonthlyCost = &monthly
+		}
 	}
 
 	return rec
diff --git a/providers/gcp/services/cloudsql/client_test.go b/providers/gcp/services/cloudsql/client_test.go
index c78f554fd..98ac60938 100644
--- a/providers/gcp/services/cloudsql/client_test.go
+++ b/providers/gcp/services/cloudsql/client_test.go
@@ -846,11 +846,12 @@ func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing
 	ctx := context.Background()
 	client, _ := NewClient(ctx, "test-project", "us-central1")
 
-	// Inject a billing mock that returns a known on-demand price. The
-	// getSQLPricing implementation applies a 15% package savings for 1yr
-	// (discount factor 0.85), so CommitmentPrice = onDemandHourly * 8760 * 0.85.
+	// Inject a billing mock with a known on-demand price and a separate
+	// commitment SKU. getSQLPricing derives CommitmentPrice from the
+	// "commitment" SKU (CommitmentPrice = commitmentHourly * 8760), so
 	// RecurringMonthlyCost must equal CommitmentPrice / 12 (one year = 12 months).
-	const hourlyRate = 0.12 // USD/h per vCPU -- representative db-n1-standard-1 value
+	const onDemandHourly = 0.12    // USD/h per vCPU -- representative db-n1-standard-1 value
+	const commitmentHourly = 0.102 // USD/h -- 1yr CUD rate from the catalog
 	mockBilling := &MockBillingService{
 		skus: &cloudbilling.ListSkusResponse{
 			Skus: []*cloudbilling.Sku{
@@ -864,7 +865,28 @@ func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing
 									{
 										UnitPrice: &cloudbilling.Money{
 											Units:        0,
-											Nanos:        int64(hourlyRate * 1e9),
+											Nanos:        int64(onDemandHourly * 1e9),
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				{
+					// Commitment SKU required by getSQLPricing: without a
+					// "commitment" SKU it errors and RecurringMonthlyCost stays nil.
+					Description:    "db-n1-standard-1 Cloud SQL commitment 1yr",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        int64(commitmentHourly * 1e9),
 											CurrencyCode: "USD",
 										},
 									},
@@ -907,11 +929,11 @@ func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *testing
 	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
 	assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Cloud SQL CUD")
 
-	// Verify the value matches CommitmentPrice / 12 exactly.
-	// estimateSQLCommitmentPrice applies a 15% savings factor for 1yr (discount=0.85).
+	// Verify the value matches CommitmentPrice / 12 exactly. CommitmentPrice is
+	// the commitment SKU's hourly rate scaled to the 1yr term total.
 	const hoursIn1yr = 8760.0
-	expectedCommitment := hourlyRate * hoursIn1yr * 0.85 // 15% savings for 1yr (see estimateSQLCommitmentPrice)
-	assert.InDelta(t, expectedCommitment/12, *rec.RecurringMonthlyCost, 1e-6)
+	expectedMonthly := commitmentHourly * hoursIn1yr / 12
+	assert.InDelta(t, expectedMonthly, *rec.RecurringMonthlyCost, 1e-6)
 }
 
 func TestCloudSQLClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) {
diff --git a/providers/gcp/services/cloudstorage/client.go b/providers/gcp/services/cloudstorage/client.go
index 07b0a3499..6a1cf2d99 100644
--- a/providers/gcp/services/cloudstorage/client.go
+++ b/providers/gcp/services/cloudstorage/client.go
@@ -525,7 +525,6 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 		term = "1yr"
 	}
 
-
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceStorage,
@@ -541,22 +540,21 @@ func (c *CloudStorageClient) convertGCPRecommendation(ctx context.Context, gcpRe
 	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
-	// correctly (issue #1022 C2).
+	// correctly (issue #1022 C2). fillStoragePricing performs the single billing
+	// lookup and populates CommitmentCost; we reuse that value below to derive
+	// RecurringMonthlyCost rather than issuing a second SKU call.
 	if rec.ResourceType != "" {
-		c.fillStoragePricing(ctx, rec, termYearsFromLabel(rec.Term))
-	}
-
-	// Populate RecurringMonthlyCost from the billing API. Cloud Storage
-	// committed-use discounts are monthly-payment commitments, so the
-	// per-month charge is CommitmentPrice / termMonths. A billing lookup
-	// failure is non-fatal: log the warning and leave RecurringMonthlyCost
-	// nil so the frontend renders "—" rather than a stale value.
-	termYears := termYearsFromLabel(rec.Term)
-	if pricing, err := c.getStoragePricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
-		log.Printf("cloudstorage: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
-	} else {
-		monthly := pricing.CommitmentPrice / float64(termYears*12)
-		rec.RecurringMonthlyCost = &monthly
+		termYears := termYearsFromLabel(rec.Term)
+		c.fillStoragePricing(ctx, rec, termYears)
+
+		// Cloud Storage committed-use discounts are monthly-payment commitments,
+		// so the per-month charge is CommitmentCost / termMonths. When the
+		// billing lookup failed, CommitmentCost stays 0 and RecurringMonthlyCost
+		// remains nil so the frontend renders "—" rather than a stale value.
+		if rec.CommitmentCost > 0 {
+			monthly := rec.CommitmentCost / float64(termYears*12)
+			rec.RecurringMonthlyCost = &monthly
+		}
 	}
 
 	return rec
diff --git a/providers/gcp/services/cloudstorage/client_test.go b/providers/gcp/services/cloudstorage/client_test.go
index c1a639dee..c8c1f2058 100644
--- a/providers/gcp/services/cloudstorage/client_test.go
+++ b/providers/gcp/services/cloudstorage/client_test.go
@@ -889,6 +889,27 @@ func TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCo
 						},
 					},
 				},
+				{
+					// Commitment SKU required by getStoragePricing: without a
+					// "commitment" SKU it errors and RecurringMonthlyCost stays nil.
+					Description:    "Standard Storage commitment 1yr in us-central1",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        20000000,
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
 			},
 		},
 	}
@@ -919,7 +940,7 @@ func TestCloudStorageClient_ConvertGCPRecommendation_PopulatesRecurringMonthlyCo
 		},
 	}
 
-	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
 	require.NotNil(t, rec)
 	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
 	assert.Greater(t, *rec.RecurringMonthlyCost, float64(0))
@@ -951,7 +972,7 @@ func TestCloudStorageClient_ConvertGCPRecommendation_BillingFailure_RecurringMon
 		},
 	}
 
-	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
 	require.NotNil(t, rec)
 	assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must remain nil when billing lookup fails")
 }
diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go
index b32f4c89f..c3002d79b 100644
--- a/providers/gcp/services/computeengine/client.go
+++ b/providers/gcp/services/computeengine/client.go
@@ -56,6 +56,17 @@ func termPlan(term string) (string, error) {
 	}
 }
 
+// termYearsFromTerm returns the commitment length in years for a term label,
+// defaulting to 1 for any 1-year or unrecognised value and 3 for 3-year labels.
+func termYearsFromTerm(term string) int {
+	switch strings.ToLower(strings.TrimSpace(term)) {
+	case "3yr", "3", "36mo":
+		return 3
+	default:
+		return 1
+	}
+}
+
 // CommitmentsService interface for commitments operations (enables mocking)
 type CommitmentsService interface {
 	List(ctx context.Context, req *computepb.ListRegionCommitmentsRequest) CommitmentsIterator
@@ -945,11 +956,18 @@ func (c *ComputeEngineClient) convertGCPRecommendation(ctx context.Context, gcpR
 
 	c.enrichRecWithPricing(ctx, rec)
 
-	// GCP Compute CUDs with PaymentOption "upfront" carry no per-month charge:
-	// the full commitment is paid at purchase time. Set RecurringMonthlyCost to
-	// a pointer-to-zero (not nil) so the frontend renders "$0" rather than "—".
-	zero := 0.0
-	rec.RecurringMonthlyCost = &zero
+	// GCP Compute CUDs are monthly-payment commitments (PaymentOption is forced
+	// to "monthly" above), so the per-month charge is CommitmentCost /
+	// termMonths. enrichRecWithPricing performs the single billing lookup and
+	// populates CommitmentCost; reuse it here rather than issuing a second call.
+	// When the billing lookup failed CommitmentCost stays 0 and we leave
+	// RecurringMonthlyCost nil (unknown) so the frontend renders "—" rather than
+	// an incorrect explicit "$0" (nil means unavailable, 0 means known-zero fee).
+	if rec.CommitmentCost > 0 {
+		termYears := termYearsFromTerm(rec.Term)
+		monthly := rec.CommitmentCost / float64(termYears*12)
+		rec.RecurringMonthlyCost = &monthly
+	}
 
 	return rec
 }
@@ -964,10 +982,7 @@ func (c *ComputeEngineClient) enrichRecWithPricing(ctx context.Context, rec *com
 	if rec.ResourceType == "" {
 		return
 	}
-	termYears := 1
-	if rec.Term == "3yr" || rec.Term == "3" {
-		termYears = 3
-	}
+	termYears := termYearsFromTerm(rec.Term)
 	pricing, err := c.getComputePricing(ctx, rec.ResourceType, c.region, termYears)
 	if err != nil {
 		log.Printf("computeengine: pricing unavailable for %s in %s (issue #1020): %v", rec.ResourceType, c.region, err)
diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go
index 8ac4cdaa4..378bfcf04 100644
--- a/providers/gcp/services/computeengine/client_test.go
+++ b/providers/gcp/services/computeengine/client_test.go
@@ -901,10 +901,11 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) {
 	assert.Equal(t, 50.5, rec.EstimatedSavings)
 	assert.Equal(t, "monthly", rec.PaymentOption,
 		"GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]")
-	// PaymentOption is "upfront" (all-upfront): no monthly charge, so
-	// RecurringMonthlyCost must be a non-nil pointer to exactly 0.
-	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil for upfront GCP CUDs")
-	assert.InDelta(t, 0.0, *rec.RecurringMonthlyCost, 1e-9)
+	// GCP Compute CUDs are monthly-billed and RecurringMonthlyCost is derived
+	// from CommitmentCost. No billing service is injected here, so the pricing
+	// lookup fails and the field stays nil (unknown) rather than an incorrect
+	// explicit 0 (nil means unavailable, 0 means a known-zero recurring fee).
+	assert.Nil(t, rec.RecurringMonthlyCost)
 }
 
 // infiniteRecommenderIterator never signals iterator.Done, used to exercise
diff --git a/providers/gcp/services/memorystore/client.go b/providers/gcp/services/memorystore/client.go
index f9cdbd5ec..e37047557 100644
--- a/providers/gcp/services/memorystore/client.go
+++ b/providers/gcp/services/memorystore/client.go
@@ -516,7 +516,6 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 		term = "1yr"
 	}
 
-
 	rec := &common.Recommendation{
 		Provider:       common.ProviderGCP,
 		Service:        common.ServiceCache,
@@ -532,22 +531,21 @@ func (c *MemorystoreClient) convertGCPRecommendation(ctx context.Context, gcpRec
 	rec.EstimatedSavings = extractGCPSavings(gcpRec)
 
 	// Thread pricing into the converter so the scorer can rank/filter GCP recs
-	// correctly (issue #1022 C2).
+	// correctly (issue #1022 C2). fillRedisPricing performs the single billing
+	// lookup and populates CommitmentCost; we reuse that value below to derive
+	// RecurringMonthlyCost rather than issuing a second SKU call.
 	if rec.ResourceType != "" {
-		c.fillRedisPricing(ctx, rec, termYearsFromLabel(rec.Term))
-	}
-
-	// Populate RecurringMonthlyCost from the billing API. Memorystore CUDs are
-	// monthly-payment commitments, so the per-month charge is
-	// CommitmentPrice / termMonths. A billing lookup failure is non-fatal:
-	// log the warning and leave RecurringMonthlyCost nil so the frontend
-	// renders "—" rather than a stale value.
-	termYears := termYearsFromLabel(rec.Term)
-	if pricing, err := c.getRedisPricing(ctx, rec.ResourceType, c.region, termYears); err != nil {
-		log.Printf("memorystore: RecurringMonthlyCost lookup failed for %s/%s: %v", rec.ResourceType, c.region, err)
-	} else {
-		monthly := pricing.CommitmentPrice / float64(termYears*12)
-		rec.RecurringMonthlyCost = &monthly
+		termYears := termYearsFromLabel(rec.Term)
+		c.fillRedisPricing(ctx, rec, termYears)
+
+		// Memorystore CUDs are monthly-payment commitments, so the per-month
+		// charge is CommitmentCost / termMonths. When the billing lookup failed,
+		// CommitmentCost stays 0 and RecurringMonthlyCost remains nil so the
+		// frontend renders "—" rather than a stale value.
+		if rec.CommitmentCost > 0 {
+			monthly := rec.CommitmentCost / float64(termYears*12)
+			rec.RecurringMonthlyCost = &monthly
+		}
 	}
 
 	return rec
diff --git a/providers/gcp/services/memorystore/client_test.go b/providers/gcp/services/memorystore/client_test.go
index e5c58dec0..1d3c7e365 100644
--- a/providers/gcp/services/memorystore/client_test.go
+++ b/providers/gcp/services/memorystore/client_test.go
@@ -780,11 +780,12 @@ func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *test
 	ctx := context.Background()
 	client, _ := NewClient(ctx, "test-project", "us-central1")
 
-	// Inject a billing mock with a known on-demand hourly price. The
-	// getRedisPricing implementation applies a 30% CUD discount for 1yr, so
-	// CommitmentPrice = onDemandHourly * 8760 * 0.70. RecurringMonthlyCost
-	// must equal CommitmentPrice / 12 (one year = 12 months).
-	const hourlyRate = 0.10 // USD/h -- representative basic Memorystore value
+	// Inject a billing mock with a known on-demand hourly price and a separate
+	// commitment SKU. getRedisPricing derives CommitmentPrice from the
+	// "commitment" SKU (CommitmentPrice = commitmentHourly * 8760), so
+	// RecurringMonthlyCost must equal CommitmentPrice / 12 (one year = 12 months).
+	const onDemandHourly = 0.10   // USD/h -- representative basic Memorystore value
+	const commitmentHourly = 0.07 // USD/h -- 1yr CUD rate from the catalog
 	client.SetBillingService(&MockBillingService{
 		skus: &cloudbilling.ListSkusResponse{
 			Skus: []*cloudbilling.Sku{
@@ -798,7 +799,28 @@ func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *test
 									{
 										UnitPrice: &cloudbilling.Money{
 											Units:        0,
-											Nanos:        int64(hourlyRate * 1e9),
+											Nanos:        int64(onDemandHourly * 1e9),
+											CurrencyCode: "USD",
+										},
+									},
+								},
+							},
+						},
+					},
+				},
+				{
+					// Commitment SKU required by getRedisPricing: without a
+					// "commitment" SKU it errors and RecurringMonthlyCost stays nil.
+					Description:    "redis-basic Cloud Memorystore Redis commitment 1yr",
+					ServiceRegions: []string{"us-central1"},
+					PricingInfo: []*cloudbilling.PricingInfo{
+						{
+							PricingExpression: &cloudbilling.PricingExpression{
+								TieredRates: []*cloudbilling.TierRate{
+									{
+										UnitPrice: &cloudbilling.Money{
+											Units:        0,
+											Nanos:        int64(commitmentHourly * 1e9),
 											CurrencyCode: "USD",
 										},
 									},
@@ -824,7 +846,7 @@ func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *test
 		},
 	}
 
-	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
 	require.NotNil(t, rec)
 
 	// RecurringMonthlyCost must be a non-nil pointer to a positive value for
@@ -832,10 +854,11 @@ func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost(t *test
 	require.NotNil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be non-nil when billing lookup succeeds")
 	assert.Greater(t, *rec.RecurringMonthlyCost, 0.0, "RecurringMonthlyCost must be positive for a monthly Memorystore CUD")
 
-	// Verify the value matches CommitmentPrice / 12 exactly.
+	// Verify the value matches CommitmentPrice / 12 exactly. CommitmentPrice is
+	// the commitment SKU's hourly rate scaled to the 1yr term total.
 	const hoursIn1yr = 8760.0
-	expectedCommitment := hourlyRate * hoursIn1yr * 0.70 // 30% discount for 1yr
-	assert.InDelta(t, expectedCommitment/12, *rec.RecurringMonthlyCost, 1e-6)
+	expectedMonthly := commitmentHourly * hoursIn1yr / 12
+	assert.InDelta(t, expectedMonthly, *rec.RecurringMonthlyCost, 1e-6)
 }
 
 func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost_BillingFailure(t *testing.T) {
@@ -851,7 +874,7 @@ func TestMemorystoreClient_ConvertGCPRecommendation_RecurringMonthlyCost_Billing
 		Content: &recommenderpb.RecommendationContent{},
 	}
 
-	rec := client.convertGCPRecommendation(ctx, gcpRec)
+	rec := client.convertGCPRecommendation(ctx, gcpRec, common.RecommendationParams{})
 	require.NotNil(t, rec)
 	assert.Nil(t, rec.RecurringMonthlyCost, "RecurringMonthlyCost must be nil when billing lookup fails")
 }

From ae95e5ea6ac8170568f9b842655479ee6f938c9d Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 10 Jul 2026 16:41:11 +0200
Subject: [PATCH 6/7] fix(gcp): correct US spelling in term-helper docstrings

golangci-lint misspell (locale US) flagged "unrecognised" in the new
termYearsFromLabel/termYearsFromTerm doc comments added by this branch.
Use "unrecognized" so the PR introduces no new lint findings.
---
 providers/gcp/services/cloudsql/client.go      | 2 +-
 providers/gcp/services/cloudstorage/client.go  | 2 +-
 providers/gcp/services/computeengine/client.go | 2 +-
 providers/gcp/services/memorystore/client.go   | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/providers/gcp/services/cloudsql/client.go b/providers/gcp/services/cloudsql/client.go
index df296b881..6a102dca0 100644
--- a/providers/gcp/services/cloudsql/client.go
+++ b/providers/gcp/services/cloudsql/client.go
@@ -510,7 +510,7 @@ func (c *CloudSQLClient) fillSQLPricing(ctx context.Context, rec *common.Recomme
 }
 
 // termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
-// integer number of years (defaults to 1 for any unrecognised value).
+// integer number of years (defaults to 1 for any unrecognized value).
 func termYearsFromLabel(term string) int {
 	if term == "3yr" || term == "3" {
 		return 3
diff --git a/providers/gcp/services/cloudstorage/client.go b/providers/gcp/services/cloudstorage/client.go
index 6a1cf2d99..5993132d0 100644
--- a/providers/gcp/services/cloudstorage/client.go
+++ b/providers/gcp/services/cloudstorage/client.go
@@ -502,7 +502,7 @@ func (c *CloudStorageClient) fillStoragePricing(ctx context.Context, rec *common
 }
 
 // termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
-// integer number of years (defaults to 1 for any unrecognised value).
+// integer number of years (defaults to 1 for any unrecognized value).
 func termYearsFromLabel(term string) int {
 	if term == "3yr" || term == "3" {
 		return 3
diff --git a/providers/gcp/services/computeengine/client.go b/providers/gcp/services/computeengine/client.go
index c3002d79b..506e0c8f6 100644
--- a/providers/gcp/services/computeengine/client.go
+++ b/providers/gcp/services/computeengine/client.go
@@ -57,7 +57,7 @@ func termPlan(term string) (string, error) {
 }
 
 // termYearsFromTerm returns the commitment length in years for a term label,
-// defaulting to 1 for any 1-year or unrecognised value and 3 for 3-year labels.
+// defaulting to 1 for any 1-year or unrecognized value and 3 for 3-year labels.
 func termYearsFromTerm(term string) int {
 	switch strings.ToLower(strings.TrimSpace(term)) {
 	case "3yr", "3", "36mo":
diff --git a/providers/gcp/services/memorystore/client.go b/providers/gcp/services/memorystore/client.go
index e37047557..f54726824 100644
--- a/providers/gcp/services/memorystore/client.go
+++ b/providers/gcp/services/memorystore/client.go
@@ -493,7 +493,7 @@ func (c *MemorystoreClient) fillRedisPricing(ctx context.Context, rec *common.Re
 }
 
 // termYearsFromLabel converts a term string such as "1yr" or "3yr" to an
-// integer number of years (defaults to 1 for any unrecognised value).
+// integer number of years (defaults to 1 for any unrecognized value).
 func termYearsFromLabel(term string) int {
 	if term == "3yr" || term == "3" {
 		return 3

From 5215a3c85fc4646f1b8627a4879a031751115cf4 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Sun, 19 Jul 2026 20:10:28 +0200
Subject: [PATCH 7/7] fix(test): fix unescaped quote in GCP CUD test assert
 message

Use a raw string literal (backtick) for the assert message that
contains square-bracket quotes around "gcp" to fix the
"missing ',' in argument list" compile error in the pre-commit
gofmt/go-vet pass.
---
 providers/gcp/services/computeengine/client_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/providers/gcp/services/computeengine/client_test.go b/providers/gcp/services/computeengine/client_test.go
index 378bfcf04..990694358 100644
--- a/providers/gcp/services/computeengine/client_test.go
+++ b/providers/gcp/services/computeengine/client_test.go
@@ -900,7 +900,7 @@ func TestComputeEngineClient_ConvertGCPRecommendation(t *testing.T) {
 	assert.Equal(t, "n1-standard-4", rec.ResourceType)
 	assert.Equal(t, 50.5, rec.EstimatedSavings)
 	assert.Equal(t, "monthly", rec.PaymentOption,
-		"GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]")
+		`GCP CUDs are billed monthly; PaymentOption must match ValidPaymentOptionsByProvider["gcp"]`)
 	// GCP Compute CUDs are monthly-billed and RecurringMonthlyCost is derived
 	// from CommitmentCost. No billing service is injected here, so the pricing
 	// lookup fails and the field stays nil (unknown) rather than an incorrect